hanzoai/dev
2
0
Fork 0
mirror of https://github.com/hanzoai/dev.git synced 2026-10-17 15:34:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(upstream): re-export crates and prune docs

Browse Source
This commit is contained in:
James Peter
2025-10-06 10:21:27 +10:00
parent 536c1aec8a
commit 175a5eac7d
66 changed files with 888 additions and 7722 deletions
Download Patch File Download Diff File Expand all files Collapse all files
DEAD_CODE_INVENTORY.md
+16 -697
View File
@@ -1,702 +1,21 @@
# Dead Code Inventory and Deletion Plan for code-rs
# Dead Code Inventory
**Generated:** 2025-10-05
**Scope:** code-rs/tui, code-rs/core, and related crates
**Purpose:** Identify and plan removal of dead compatibility scaffolding, orphaned modules, and unused code
**Updated:** October 6, 2025
---
The heavyweight inventory used during the initial cleanup is no longer needed. This lightweight version tracks only the active watchlist so we can focus on forward progress.
## Executive Summary
## What’s Done
- Removed ~35K lines of legacy tests and deleted 106 files tied to the old codex execution flow.
- Dropped the `legacy_tests` and `vt100-tests` feature flags plus their helper modules.
- Deleted obsolete TUI overlay/backtrack code and archived historic planning notes under `docs/archive/`.
- Converted six infrastructure crates to thin upstream re-exports (`code-ansi-escape`, `code-backend-client`, `code-cloud-tasks-client`, `code-execpolicy`, `code-git-apply`, `code-linux-sandbox`).
This document provides a comprehensive inventory of dead or unused code in the code-rs codebase, organized by category with specific deletion plans and verification steps.
## Current Watchlist
- Review `code-rs/core` while the upstream reuse audit runs; flag any modules that can be replaced wholesale once identical.
- Track `code-rs/tui` files still carrying `#![allow(dead_code)]` (mostly streaming helpers) and either legitimize or delete them as part of the history subsystem cleanup.
- Confirm the rebuilt test suites cover vt100 rendering once Phase 3 lands so no dead fixtures sneak back in.
**Status as of 2025-10-05:** Phase 1 cleanup complete. Feature flag audit complete. Pending follow-up on TUI overlay/backtrack modules.
### Key Findings
- ✅ **~35,000 lines** of dead test code (106 test files across 8 crates) — **REMOVED**
- ✅ **6 orphaned modules** totaling ~72,700 bytes — **REMOVED** (see Section 2.1)
- ✅ **Feature flag audit complete** — `code-fork` feature is properly used and enabled by default
- ✅ **Core modules audit complete** — All modules (`codex/`, `unified_exec/`, `exec_command/`) are actively used
- ⏳ **TUI modules audit** — Streaming controller/markdown modules still under review; overlay stack (`pager_overlay.rs`, `backtrack_helpers.rs`, `resume_picker.rs`) deleted
- **9 modules** marked with `#![allow(dead_code)]` at file level — **ACCEPTABLE** (mostly helpers and utilities)
- ✅ `vt100-tests` feature flag removed from `code-rs/tui/Cargo.toml` (2025-10-05)
- **0 orphaned prompt files** (all are actively used, documented in `docs/maintenance/prompt-architecture.md`)
- **Stale comments** about removed code — **CLEANED UP**
---
## Completed Removals
### Phase 1: Legacy Test Infrastructure (✅ COMPLETED)
**Status:** All legacy test modules and feature flags removed as of 2025-10-05
**Items Removed:**
- `legacy_tests` feature flag from `code-rs/tui/Cargo.toml`
- All 27 gated test modules (~11,232 lines)
- Orphaned modules: `app_backtrack.rs`, `custom_terminal.rs`, `scroll_view.rs`, `text_block.rs`, `transcript_app.rs`, `pager_overlay.rs`, `backtrack_helpers.rs`, `resume_picker.rs`
- Legacy `exec_cell/render.rs`
- Introduced new smoke scaffold at `code-rs/tui/tests/ui_smoke.rs`
**Verification:** Build passes with `./build-fast.sh --workspace code`
---
## Category 1: Feature Flags (ARCHIVED - COMPLETED)
### 1.1 legacy_tests Feature Flag ✅ REMOVED
**Location:** `code-rs/tui/Cargo.toml:20` (DELETED)
**Status:** REMOVED - Feature and all gated code deleted
**Scope:**
- 2 dedicated test files: 4,985 lines
- 23 modules with gated test code: ~6,247 lines
- Total: **~11,232 lines of dead code**
**Files Affected:**
```
code-rs/tui/src/chatwidget/tests.rs (4,634 lines)
code-rs/tui/src/chatwidget/tests_retry.rs (351 lines)
code-rs/tui/src/text_formatting.rs (~164 lines of tests)
code-rs/tui/src/text_processing.rs (~24 lines of tests)
code-rs/tui/src/updates.rs (~23 lines of tests)
code-rs/tui/src/status_indicator_widget.rs (~53 lines of tests)
code-rs/tui/src/pager_overlay.rs (~209 lines of tests)
code-rs/tui/src/markdown_renderer.rs (~75 lines of tests)
code-rs/tui/src/markdown_stream.rs (~499 lines of tests)
code-rs/tui/src/markdown.rs (~251 lines of tests)
code-rs/tui/src/live_wrap.rs (~81 lines of tests)
code-rs/tui/src/insert_history.rs (~80 lines of tests)
code-rs/tui/src/exec_command.rs (~17 lines of tests)
code-rs/tui/src/diff_render.rs (~155 lines of tests)
code-rs/tui/src/clipboard_paste.rs (~115 lines of tests)
code-rs/tui/src/user_approval_widget.rs (~138 lines of tests)
code-rs/tui/src/backtrack_helpers.rs (~41 lines of tests)
code-rs/tui/src/streaming/controller.rs (~180 lines of tests)
code-rs/tui/src/bottom_pane/textarea.rs (~914 lines of tests)
code-rs/tui/src/bottom_pane/mod.rs (~256 lines of tests)
code-rs/tui/src/bottom_pane/chat_composer.rs (~751 lines of tests)
code-rs/tui/src/bottom_pane/chat_composer_history.rs (~61 lines of tests)
code-rs/tui/src/bottom_pane/command_popup.rs (~319 lines of tests)
code-rs/tui/src/bottom_pane/scroll_state.rs (~28 lines of tests)
code-rs/tui/src/bottom_pane/approval_modal_view.rs (~37 lines of tests)
```
**Deletion Plan:**
1. **Delete entire test files:**
```bash
rm code-rs/tui/src/chatwidget/tests.rs
rm code-rs/tui/src/chatwidget/tests_retry.rs
```
2. **Remove gated test modules from each file:**
Search for `#[cfg(all(test, feature = "legacy_tests"))]` and delete the entire module block in each of the 23 files listed above.
3. **Remove feature flag from Cargo.toml:**
```bash
# Edit code-rs/tui/Cargo.toml, delete lines 19-20:
# # Disable legacy tests by default; enable with `--features legacy_tests` if needed.
# legacy_tests = []
```
4. **Remove test-only helper functions:**
- `simulate_stream_markdown_for_tests()` in `markdown_stream.rs`
- `CommandPopup::new()` test constructor
- `ChatWidget::test_for_request()` constructor
**Verification Steps:**
```bash
# 1. Verify feature is not in CI/CD
grep -r "legacy_tests" .github/ scripts/ justfile
# 2. Attempt build before deletion (should succeed)
cd code-rs/tui && cargo build
# 3. Perform deletion
# 4. Verify build still succeeds
cargo build
# 5. Verify tests run (non-legacy tests)
cargo test
# 6. Check for any remaining references
rg "legacy_tests" code-rs/
```
**Risk:** LOW - Code is already broken and unused
---
### 1.2 vt100-tests Feature Flag ✅ REMOVED (2025-10-05)
**Location:** `code-rs/tui/Cargo.toml` (lines removed)
**Status:** Feature flag deleted; remaining vt100 usage is runtime-only (`chatwidget/terminal.rs`).
**Notes:**
- VT100-based snapshot tests now live only in the upstream mirror (`codex-rs`).
- Re-introduce the feature if/when fork-specific vt100 tests return.
---
## Category 2: Orphaned Modules and Files (ARCHIVED - COMPLETED)
### 2.1 Confirmed Orphaned Files (code-rs/tui) ✅ REMOVED
#### 2.1.1 text_block.rs ✅ REMOVED
**Location:** `code-rs/tui/src/text_block.rs` (DELETED)
**Size:** 278 bytes
**Status:** REMOVED - File and lib.rs comment deleted
**Deletion Plan:**
```bash
rm code-rs/tui/src/text_block.rs
# Also remove commented line from code-rs/tui/src/lib.rs:73
```
**Verification:**
```bash
rg "text_block" code-rs/tui/
cargo build -p code-tui
```
**Risk:** NONE - Already acknowledged as orphaned
---
#### 2.1.2 scroll_view.rs ✅ REMOVED
**Location:** `code-rs/tui/src/scroll_view.rs` (DELETED)
**Size:** 4,529 bytes
**Status:** REMOVED - File and lib.rs comment deleted
**Deletion Plan:**
```bash
rm code-rs/tui/src/scroll_view.rs
# Also remove commented line from code-rs/tui/src/lib.rs:63
```
**Verification:**
```bash
rg "scroll_view" code-rs/tui/
cargo build -p code-tui
```
**Risk:** NONE - Already acknowledged as orphaned
---
#### 2.1.3 app_backtrack.rs ✅ REMOVED
**Location:** `code-rs/tui/src/app_backtrack.rs` (DELETED)
**Size:** 14,257 bytes
**Status:** REMOVED - File deleted
**Deletion Plan:**
```bash
rm code-rs/tui/src/app_backtrack.rs
```
**Verification:**
```bash
# Check for any references to BacktrackState
rg "BacktrackState" code-rs/tui/
rg "app_backtrack" code-rs/tui/
cargo build -p code-tui
```
**Risk:** LOW - No module declaration found
---
#### 2.1.4 resume_picker.rs ✅ REMOVED
**Location:** `code-rs/tui/src/resume_picker.rs` (DELETED)
**Size:** 37,218 bytes
**Status:** REMOVED - Alternate-screen picker superseded by inline resume prompt
**Verification:**
```bash
rg "resume_picker" code-rs/tui/
```
**Risk:** NONE - No code paths referenced the module
---
#### 2.1.5 custom_terminal.rs ✅ REMOVED
**Location:** `code-rs/tui/src/custom_terminal.rs` (DELETED)
**Size:** 22,672 bytes
**Status:** REMOVED - Deleted along with legacy_tests
**Note:** Was only referenced by deleted `resume_picker.rs` and `chatwidget/tests.rs`
**Deletion Plan:**
**Option A:** Delete if legacy_tests are removed:
```bash
rm code-rs/tui/src/custom_terminal.rs
```
**Option B:** Add module declaration if needed:
```rust
// In code-rs/tui/src/lib.rs, add:
#[cfg(test)]
mod custom_terminal;
```
**Verification:**
```bash
rg "custom_terminal" code-rs/tui/
cargo build -p code-tui
cargo test -p code-tui
```
**Risk:** MEDIUM - Used by test code, but those tests are broken
**Recommendation:** Delete along with legacy_tests removal
---
#### 2.1.6 exec_cell/ directory ✅ REMOVED
**Location:** `code-rs/tui/src/exec_cell/` (DELETED)
**Contents:** `render.rs` (16,444 bytes)
**Status:** REMOVED - Directory deleted
**Note:** The actual ExecCell is in `history_cell/exec.rs`; this was old/duplicate code
**Deletion Plan:**
```bash
rm -rf code-rs/tui/src/exec_cell/
```
**Verification:**
```bash
rg "exec_cell" code-rs/tui/
rg "crate::exec_cell" code-rs/tui/
cargo build -p code-tui
```
**Risk:** LOW - No references found
---
### 2.2 Module Declaration Issues (code-rs/core)
#### 2.2.1 tasks/ directory
**Location:** `code-rs/core/src/tasks/`
**Contents:** `mod.rs`, `compact.rs`, `regular.rs`, `review.rs`
**Status:** USED but not declared as module
**Issue:** Code in `state/turn.rs` references `use crate::tasks::SessionTask;` but `mod tasks;` is missing from lib.rs
**Action Required:** ADD MODULE DECLARATION (not removal)
```rust
// In code-rs/core/src/lib.rs, add:
pub mod tasks;
```
**Verification:**
```bash
cargo build -p code-core
cargo test -p code-core
```
**Risk:** NONE - This fixes a bug
---
#### 2.2.2 tests/suite/otel.rs
**Location:** `code-rs/core/tests/suite/otel.rs`
**Status:** Test file not declared in `tests/suite/mod.rs`
**Action Required:** Either add to mod.rs or delete
**Option A:** Add to suite:
```rust
// In code-rs/core/tests/suite/mod.rs, add:
mod otel;
```
**Option B:** Delete if unused:
```bash
rm code-rs/core/tests/suite/otel.rs
```
**Verification:**
```bash
cargo test -p code-core --test suite
```
---
## Category 3: Modules Marked #[allow(dead_code)]
**Status:** Partially cleaned; pager overlay stack and backtrack helpers remain under review.
### 3.1 Remaining Candidates for Review (code-rs/tui)
| File | Size | Status |
|------|------|--------|
| `src/streaming/controller.rs` | 18,450 B | `#![allow(dead_code)]` — PENDING REVIEW |
| `src/streaming/mod.rs` | 3,952 B | `#![allow(dead_code)]` — PENDING REVIEW |
| `src/markdown_stream.rs` | 41,063 B | `#![allow(dead_code)]` — PENDING REVIEW |
| `src/markdown.rs` | 28,666 B | `#![allow(dead_code)]` — PENDING REVIEW |
| ✅ ~~`src/transcript_app.rs`~~ | ~~9,904 B~~ | **REMOVED** |
| `src/bottom_pane/list_selection_view.rs` | ? | `#![allow(dead_code)]` — PENDING REVIEW |
| `src/bottom_pane/paste_burst.rs` | ? | `#![allow(dead_code, unused_imports, unused_variables)]` — PENDING REVIEW |
**Remaining:** ~70,000+ bytes requiring investigation
### 3.2 Core Modules (code-rs/core)
| File | Status |
|------|--------|
| `src/acp.rs` | `#![allow(dead_code)]` |
| `src/function_tool.rs` | `#[allow(dead_code)]` on module |
**Investigation Plan:**
For each file:
1. **Check for external references:**
```bash
# Example for pager_overlay.rs
rg "pager_overlay" code-rs/ --type rust | grep -v "src/pager_overlay.rs"
rg "PagerOverlay" code-rs/ --type rust | grep -v "src/pager_overlay.rs"
```
2. **Try compilation without file:**
```bash
# Temporarily rename file
mv src/pager_overlay.rs src/pager_overlay.rs.bak
cargo build -p code-tui
# If build succeeds, the file is safe to delete
```
3. **Check git history:**
```bash
git log --oneline --follow src/pager_overlay.rs | head -20
```
**Verification Process:**
```bash
# For each potentially dead module:
# 1. Search for usage
rg "ModuleName" code-rs/
# 2. Comment out module declaration in lib.rs
# (or rename file to .bak)
# 3. Build
cargo build
# 4. Test
cargo test
# 5. If successful, proceed with deletion
```
**Risk:** MEDIUM - Requires individual verification of each module
---
## Category 4: Stale Comments
### 4.1 Comments About Removed Code (Safe to Delete)
These are comments documenting code that has already been removed:
**code-rs/core/src/codex.rs:**
- Line 9010: `// removed upstream exit_review_mode helper: not used in fork`
- Lines 2539, 2542, 2554, 2556, 2564: `// (debug removed)`
- Line 2884: `// (submission diagnostics removed)`
**code-rs/core/src/client.rs:**
- Line 645: `// duplicate of earlier helpers removed during merge cleanup`
**code-rs/tui/src/chatwidget.rs:**
- Line 1235: `// Removed legacy turn-window logic; ordering is strictly global.`
- Line 2674: `// Legacy helper removed: streaming now requires explicit sequence numbers.`
- Line 10905: `// Legacy show_agents_settings_ui removed — overview/Direct editors replace it`
- Line 15695: `// removed legacy ensure_stream_order_key; strict variant is used instead`
**code-rs/tui/src/chatwidget/exec_tools.rs:**
- Line 1023: `// Stable ordering now inserts at the correct position; these helpers are removed.`
**Deletion Plan:**
```bash
# Manually remove these comments from the files listed above
# This is a code cleanup task with no functional impact
```
**Verification:** Visual inspection + `cargo build`
**Risk:** NONE
---
### 4.2 Comments to Keep (Important Documentation)
These comments document architectural decisions and should be retained:
- `code-rs/core/src/codex.rs:3843` - Documents intentional review flow divergence
- `code-rs/core/src/codex.rs:3545` - Documents upstream protocol changes
- `code-rs/core/src/client.rs:1212-1214` - Documents where to find removed helpers if needed
- `code-rs/core/src/exec.rs:35` - Documents architectural change
- All "legacy" config compatibility comments in `config_types.rs`
---
## Category 5: Documentation Files
### 5.1 Planning/Design Documents (ARCHIVED)
**Location:** `docs/archive/tui-migrations/`
**Files:**
- `plain_loading_wait_migration.md`
- `renderer_cache.md`
- `stream_exec_assistant_migration.md`
**Status:** Archived for historical reference (no active code references).
**Verification:**
```bash
ls docs/archive/tui-migrations/
```
**Risk:** LOW - Documentation only
---
### 5.2 Active Documentation (Keep)
**code-rs/tui/HISTORY_CELLS_PLAN.md** - Active planning doc, keep
**code-rs/tui/styles.md** - Developer reference, keep (or move to docs/)
---
## Category 6: Prompt Files
### Status: ALL ACTIVE
No orphaned prompt files found. All prompt files have confirmed `include_str!()` usage:
- ✅ `code-rs/core/gpt_5_code_prompt.md` - Used by gpt-5-codex models
- ✅ `code-rs/core/prompt.md` - Base instructions for all models
- ✅ `code-rs/core/prompt_coder.md` - Additional developer instructions
- ✅ `code-rs/core/review_prompt.md` - Review mode functionality
- ✅ `code-rs/tui/prompt_for_init_command.md` - /init slash command
- ✅ `code-rs/core/templates/compact/prompt.md` - Context compaction
- ✅ `code-rs/core/templates/compact/history_bridge.md` - Context compaction
**Action:** None required
---
## Deletion Priority Matrix
### ✅ P0 - Immediate (Zero Risk) — COMPLETED
1. ✅ Delete `text_block.rs` + comment in lib.rs
2. ✅ Delete `scroll_view.rs` + comment in lib.rs
3. ⚠️ Delete stale comments in codex.rs, client.rs, chatwidget.rs — PARTIALLY DONE
4. ⚠️ Fix module declaration for `tasks/` directory — NEEDS VERIFICATION
**Cleanup achieved:** ~5,000 bytes + improved code clarity
---
### ✅ P1 - High Priority (Low Risk) — COMPLETED
1. ✅ Remove `legacy_tests` feature flag and all gated code (~11,232 lines)
2. ✅ Delete `app_backtrack.rs`
3. ✅ Delete `resume_picker.rs`
4. ✅ Delete `custom_terminal.rs`
5. ✅ Delete `exec_cell/` directory
6. ✅ Remove `vt100-tests` feature flag from code-rs/tui
**Cleanup achieved:** ~11,300 lines + ~90KB
---
### P2 - Medium Priority (Requires Investigation) — IN PROGRESS
1. Investigate and remove modules marked `#![allow(dead_code)]`
- ✅ ~~`pager_overlay.rs`~~ REMOVED
- ✅ ~~`backtrack_helpers.rs`~~ REMOVED
- ✅ ~~`transcript_app.rs`~~ REMOVED
- ⚠️ `streaming/` directory modules — PENDING REVIEW
- ⚠️ `markdown.rs` and `markdown_stream.rs` — PENDING REVIEW
2. ⚠️ Fix or delete `tests/suite/otel.rs` — NEEDS INVESTIGATION
3. ✅ Archive or delete `agent_tasks/` directory — ARCHIVED to `docs/archive/tui-migrations/`
**Remaining cleanup estimate:** ~102KB
---
### P3 - Low Priority (Documentation)
1. Review and archive old planning docs in `agent_tasks/`
2. Consider moving `styles.md` to docs/ directory
**Estimated cleanup:** Organizational only
---
## Verification Checklist
After each deletion phase:
```bash
# 1. Build succeeds
cargo build --workspace
# 2. Tests pass (non-legacy)
cargo test --workspace
# 3. Clippy is happy
cargo clippy --workspace -- -D warnings
# 4. Format is correct
cargo fmt --check
# 5. No new warnings
cargo build --workspace 2>&1 | grep -i warning
# 6. Git status is clean (except intended changes)
git status
```
---
## Rollback Plan
Before any deletions:
```bash
# Create a backup branch
git checkout -b backup-before-dead-code-cleanup
git push origin backup-before-dead-code-cleanup
# Create working branch
git checkout -b cleanup-dead-code-phase-1
# Perform deletions with individual commits
git commit -m "Remove legacy_tests feature and gated code"
git commit -m "Remove orphaned modules: text_block, scroll_view"
# etc.
# If issues arise, revert specific commits or reset to backup branch
```
---
## Success Metrics
**Phase 1 Results (as of 2025-10-05):**
- **Lines of code removed:** ~35,000+ lines ✅ (far exceeded target)
- **Files removed:** 106 test files + 6 orphaned modules ✅ (far exceeded target)
- **Build time improvement:** Verified with `./build-fast.sh --workspace code` - 1m 30s ✅
- **Code complexity reduction:** Significantly fewer modules to maintain ✅
- **Feature flag audit:** `code-fork` properly used, no unused flags ✅
- **Module audit:** All core and TUI modules actively used ✅
- **Documentation:** Comprehensive migration docs created in `docs/migration/` and `docs/maintenance/` ✅
**Audit Results (2025-10-05):**
- ✅ `code-fork` feature flag: Enabled by default, gates 12 fork-specific TUI extensions (foundation.rs, tui_event_extensions.rs, etc.)
- ✅ `code-rs/core` modules: All subdirectories (`codex/`, `unified_exec/`, `exec_command/`) actively used
- ✅ `code-rs/tui` modules: All remaining modules actively used, no orphaned code detected
- ✅ `#![allow(dead_code)]` usage: 9 files with file-level allows (acceptable for test helpers, utilities, and compatibility wrappers)
**Remaining Work:**
- ✅ No critical dead code remaining
- 📋 Future: Monitor for new dead code during upstream merges
- 📋 Future: Port 9 critical tests per `docs/migration/legacy-tests-retirement.md`
---
## Audit Summary (2025-10-05)
### Feature Flags Audit
**`code-fork` feature flag (code-rs/tui):**
- Status: ✅ ACTIVE AND PROPERLY USED
- Enabled by default in `code-rs/tui/Cargo.toml`
- Gates 12 fork-specific files:
- `src/foundation.rs` - Stable import wrappers for fork-specific code
- `src/tui_event_extensions.rs` - Rate limit and browser screenshot event helpers
- `src/rate_limits_view.rs` - Rate limits visualization
- `src/bottom_pane/approval_ui.rs` - Approval UI components
- `src/history/compat.rs` - Fork-specific history compatibility
- Various files with selective gating for fork-specific features
**`vt100-tests` feature flag:**
- Status: ✅ RETAINED
- Purpose: Enable VT100 emulator-based tests (currently no tests use it, but scaffolding preserved for future)
**`debug-logs` and `dev-faults` feature flags:**
- Status: ✅ ACTIVE
- Purpose: Debug logging and fault injection for development
**No unused feature flags detected.**
### Module Usage Audit
**code-rs/core modules:**
- ✅ `src/codex/` - Actively used for compact conversation format
- ✅ `src/unified_exec/` - Actively used for unified executor implementation
- ✅ `src/exec_command/` - Actively used for command execution and session management
- ✅ All modules properly declared in `lib.rs` and actively used
**code-rs/tui modules:**
- ✅ All modules under `src/chatwidget/`, `src/bottom_pane/`, `src/history_cell/` actively used
- ✅ `src/foundation.rs`, `src/tui_event_extensions.rs` provide fork-specific extensions
- ✅ No orphaned modules detected
**`#![allow(dead_code)]` usage:**
- 9 files use file-level `#![allow(dead_code)]`
- Acceptable uses: test helpers (`smoke_helpers.rs`), utilities, compatibility wrappers
- No action required
### Related Documentation
- Migration plan: `docs/migration/legacy-tests-retirement.md`
- Prompt architecture: `docs/maintenance/prompt-architecture.md`
- Upstream tracking: `docs/maintenance/upstream-diff.md`
- TUI smoke tests: `docs/migration/tui-smoke-notes.md`
---
## Notes for Future Maintenance
1. **Avoid `#![allow(dead_code)]` at file level** - This suppresses important warnings and makes it hard to identify truly dead code. Use at item level only when justified.
2. **Remove code instead of commenting it out** - Git preserves history; commented-out module declarations (like `text_block.rs` in lib.rs) create confusion.
3. **Clean up stale comments** - Comments about "removed" code should be deleted along with the code.
4. **Keep feature flags clean** - Remove unused feature flags promptly to avoid confusion.
5. **Test infrastructure hygiene** - If tests don't compile, either fix them or delete them. Don't hide behind feature flags.
---
**End of Document**
## Quick Verification Tips
- `rg "#!\[allow(dead_code)" code-rs/tui` – sanity-check remaining allowances.
- `./build-fast.sh` – must stay warning-free after any cleanup.
- Note findings directly here so the roadmap can link to a single source of truth.
INVESTIGATION_SUMMARY.md
-116
View File
@@ -1,116 +0,0 @@
# MCP Client Upstream Reuse Investigation Summary
**Branch**: `code-claude-investigate-reusing-codex-rs-mcp-client`
**Date**: 2025-10-05
**Investigator**: Claude Code Agent
## Key Findings
### **RECOMMENDATION: Reuse upstream codex-rs MCP components directly** ✅
The investigation reveals **minimal divergence** between code-rs and codex-rs implementations:
#### 1. MCP Client (`mcp-client`)
- **Only difference**: 1-line buffer size change (1MB vs default)
- **File similarity**: 476 vs 475 lines (99.8% identical)
- **Action**: Test if upstream buffer is sufficient; if not, contribute config option upstream
#### 2. Responses API Proxy (`responses-api-proxy`)
- **Difference**: ZERO - byte-for-byte identical logic
- **Only variation**: Package/binary naming (`code-*` vs `codex-*`)
- **Action**: Use upstream with build-time binary rename OR accept upstream naming
#### 3. Process Hardening (`process-hardening`)
- **Difference**: Cosmetic code style only (functionally identical)
- **Action**: Use upstream directly via re-export
## Implementation Strategy
**Phase 1 (Immediate)**:
- Switch to `codex-mcp-client` dependency
- Test with default buffer size
- Use `codex-process-hardening` via re-export
**Phase 2 (1-2 weeks)**:
- Propose upstream PR for buffer configuration (if needed)
- Migrate responses-api-proxy with binary rename
- Establish automated upstream monitoring
**Phase 3 (Ongoing)**:
- Weekly upstream change checks
- Participate in upstream development
- Maintain zero-patch strategy
## Detailed Documentation
1. **[upstream-mcp-reuse-strategy.md](docs/upstream-mcp-reuse-strategy.md)**
- Comprehensive analysis of all differences
- Three proposed strategies (A: Direct Dependency, B: Feature-Flag, C: Automated Sync)
- Risk assessment and mitigation
- Maintenance checklist
2. **[upstream-mcp-implementation-guide.md](docs/upstream-mcp-implementation-guide.md)**
- Step-by-step migration instructions
- Code examples for wrapper/config approach
- Testing and validation procedures
- Upstream contribution workflow
- Rollback plan
## Fork-Specific Tweaks Required
| Component | Tweak | Complexity | Mitigation |
|-----------|-------|------------|------------|
| Buffer size | 1MB vs default | Trivial | Config option (upstream PR) or wrapper |
| Binary names | `code-*` vs `codex-*` | Low | Build script rename OR accept upstream |
| Process hardening | Re-export naming | Trivial | `pub use codex_process_hardening as code_process_hardening` |
## Upstream Parity Maintenance
**Weekly**:
- Check codex-rs commits to MCP components
- Run integration tests with latest upstream
- Review upstream issues/PRs
**Per-Release**:
- Review changelog for breaking changes
- Full test suite validation
- Update documentation if needed
**Monthly**:
- Performance benchmarks
- Security audit review
- Evaluate sunset of workarounds
## Risk Assessment: LOW ✅
| Risk | Likelihood | Impact | Status |
|------|------------|--------|--------|
| API breakage | Low | High | Mitigated: Pin versions, CI tests |
| Buffer regression | Medium | Medium | Mitigated: Integration tests |
| Upstream abandonment | Very Low | High | Fallback: Current fork exists |
## Conclusion
**The fork is unnecessary.** Code-rs can safely depend on upstream codex-rs MCP components with:
- Zero maintenance burden for core functionality
- Automatic security updates
- Trivial customization via wrapper or upstream contribution
**Recommended Action**: Implement Strategy A (Direct Upstream Dependency) immediately.
---
## Files Modified/Created
- `code-rs/docs/upstream-mcp-reuse-strategy.md` - Comprehensive analysis
- `code-rs/docs/upstream-mcp-implementation-guide.md` - Implementation guide
- `INVESTIGATION_SUMMARY.md` - This summary
## Next Steps
1. Review documents with team
2. Decide on binary naming convention
3. Create test branch with upstream dependencies
4. Run full test suite
5. Benchmark performance with actual workloads
6. If successful, merge and close fork divergence
ROADMAP.md
+30 -168
View File
@@ -1,177 +1,39 @@
# Roadmap
## Objectives
- Protect our ability to absorb upstream changes quickly while letting `code-rs` own the fork-only UX, policy, and tooling layers.
- Remove legacy compatibility scaffolding (dead modules, feature flags, tests, assets) so the tree matches the product we actually ship.
- Rebuild the test suite around lightweight smoke/targeted coverage that reflects `code-rs` behavior today instead of the old codex execution model.
- Institutionalize upstream tracking, reuse decisions, and maintenance checklists so each merge is predictable.
- Standardize parallel-agent workflows (one worktree per track) to keep large refactors moving without blocking on long-running investigations.
## Current Objectives
- Reuse upstream `codex-*` crates and modules wherever behavior matches, keeping fork-only UX, policy, and tooling layers in `code-rs`.
- Rebuild a lean, representative test suite that exercises current workflows instead of legacy codex paths.
- Maintain a predictable cadence for upstream diffs and documentation so merges stay low-drama.
## Upstream Reuse Strategy
| Area | Current Delta | Strategy | Status & Actions |
| --- | --- | --- | --- |
| `mcp-client` | Rename + larger stdout buffer. | Adopt upstream crate directly, keep fork-only buffer change via wrapper if still required. | ✅ **Completed (2025-10-05)**: Re-exported upstream `codex-mcp-client` as `code-mcp-client` with thin wrapper preserving buffer settings. |
| `responses-api-proxy` | Rename + larger API key buffer + extra test. | Consume upstream crate, keep buffer tweak behind feature or downstream patch. | ✅ **Completed (2025-10-05)**: Re-exported upstream `codex-responses-api-proxy` as `code-responses-api-proxy` with minimal fork-specific changes. |
| `exec` | Substantial policy/output changes. | Build adapters for upstream executor where possible; retain fork-specific approval pipeline. | Catalogue fork-only code paths (policy prompts, streaming) and design adapter boundaries in `code-core`. |
| `login` | Auth flow tweaks, HTML assets, approval prompts. | Keep forked implementation, but cross-check with upstream helpers for OAuth updates. | Diff upstream releases regularly; cherry-pick auth fixes while preserving fork UX. |
| `protocol` | Added MCP protocol types. | Maintain fork version; push reusable schema changes upstream if possible. | Document any upstream schema additions needed for fork behavior. |
| `core`, `mcp-server`, `tui`, `app-server` | Massive divergence (fork-only features). | Treat as fork-owned; only pull upstream when changes unblock us. | Record upstream changes that impact prompts/executor and implement targeted integrations. |
## Recent Milestones *(Completed October 5, 2025)*
- Retired the legacy vt100 and executor test suites (~35K lines removed, 106 files).
- Converted six infrastructure crates (`code-ansi-escape`, `code-backend-client`, `code-cloud-tasks-client`, `code-execpolicy`, `code-git-apply`, `code-linux-sandbox`) into thin re-exports of their upstream counterparts.
- Pruned obsolete TUI overlay/backtrack modules and archived earlier planning artifacts.
## Codebase Cleanup Plan
Supporting detail: `DEAD_CODE_INVENTORY.md`
## Active Initiatives
### Completed (as of 2025-10-05)
- ✅ Removed legacy test suite (`legacy_tests` feature and old codex execution tests)
- Deleted ~106 test files across 8 crates (app-server, apply-patch, chatgpt, cli, core, exec, login, mcp-server)
- Removed ~35,000 lines of outdated test code
- See `docs/migration/legacy-tests-retirement.md` for porting plan
- ✅ Removed legacy TUI test harness files (`chatwidget/tests.rs`, `chatwidget/tests_retry.rs`) and the orphaned `exec_cell/render.rs`
- ✅ Removed defunct TUI overlay/backtrack modules (`pager_overlay.rs`, `backtrack_helpers.rs`, `resume_picker.rs`)
- ✅ Archived `code-rs/tui/agent_tasks/` planning docs under `docs/archive/tui-migrations/`
- ✅ Deleted `code-rs/tui/src/compat.rs` ratatui adapter layer; import ratatui types directly
- ✅ Removed all `cfg(feature = "legacy_tests")` blocks
- ✅ Removed `vt100-tests` feature flag from `code-rs/tui/Cargo.toml` (2025-10-05)
- ✅ Created comprehensive migration documentation in `docs/migration/` and `docs/maintenance/`
### 1. Upstream Reuse Audit
- Catalogue modules in `code-rs/core`, `code-rs/exec`, and `code-rs/tui` that are still identical to upstream.
- Replace identical modules with `pub use codex_*::…` re-exports; retain fork-only policy, approval, and UX layers locally.
- After each pass, capture remaining divergences in-line with the module list and update this roadmap.
### Needs Follow-Up
- ⏳ Audit remaining TUI modules marked `#![allow(dead_code)]` (streaming controller, markdown stack)
- ⏳ Reconcile vt100 rendering coverage — upstream snapshots live only in `codex-rs`
### 2. Test Suite Rebuild *(October 20 – November 30, 2025)*
- Port six core runtime tests (`model_tools.rs`, `tool_harness.rs`, `tools.rs`, `read_file.rs`, `view_image.rs`, `unified_exec.rs`).
- Port three TUI rendering tests with deterministic fixtures and golden outputs.
- Expand smoke harness coverage for exec, approval, and tool flows using the helpers documented in `TEST_SUITE_RESET.md`.
### Active Work
1. **Prune unused modules and assets**
- 📋 Audit `code-rs/tui` for modules ported from upstream but unused (e.g., status widgets, legacy assets).
- 📋 Review `code-rs/core` for upstream executor skeletons left over from earlier merges (`tools/`, `executor/`, etc.) and remove or quarantine them.
2. **Consolidate prompts and config**
- ✅ Audited prompt files; snapshot recorded in `docs/maintenance/prompt-architecture.md`; no unused prompts detected.
- 📋 Monitor prompt changes during upstream merges per `docs/maintenance/upstream-diff.md`
3. **Simplify feature flags and cfgs**
- 📋 Audit remaining fork-only flags (e.g., `code-fork`) and `code-*` prefix consistency.
### 3. Maintenance & Tooling Ready State
- Run the first-Monday diff cadence with `scripts/upstream-merge/diff-crates.sh --all`; escalate to critical analysis only when changes warrant it.
- Track prompt/config deltas through `docs/maintenance/upstream-diff.md` so downstream teams can react quickly.
- Monitor thin wrapper crates for API drift and keep their configuration nubs documented in the crate READMEs.
## Test Suite Reset
Supporting detail: `TEST_SUITE_RESET.md`, `docs/migration/legacy-tests-retirement.md`
## Next Actions
- [ ] Prepare a fresh `codex-rs` checkout (or update the mirror) for upcoming module comparisons.
- [ ] Diff `code-rs/core/src/history/` against upstream and replace identical modules with re-exports.
- [ ] Kick off Phase 2 of the test rebuild by porting `model_tools.rs`.
- [ ] Schedule the next upstream diff review on October 13, 2025 (second-Monday merge planning hold).
### Phase 1: Completed (2025-10-05)
- ✅ Removed `legacy_tests` feature and deleted unit tests targeting the old codex execution flow (~106 files, 35,000+ lines).
- ✅ Dropped vt100 fixtures and binary-size snapshots that no longer reflect current UI behavior.
- ✅ Verified TUI smoke tests passing (all bottom panes, approval flow, markdown/code streaming, auto-drive sessions).
- ✅ Build verification passed with no regressions.
### Phase 2-3: In Progress (Oct 20 - Nov 30, 2025)
**Focus:** Port critical test coverage and expand smoke tests.
**Known Blockers (9 legacy-only test files requiring replacement):**
- `codex-rs/core/tests/suite/`: `model_tools.rs`, `tool_harness.rs`, `tools.rs`, `read_file.rs`, `view_image.rs`, `unified_exec.rs` (6 files)
- `codex-rs/tui/tests/suite/`: `vt100_history.rs`, `vt100_live_commit.rs`, `status_indicator.rs` (3 files)
**Timeline & Actions:**
1. **Phase 2 (Oct 20-31)**: Core Runtime Tests
- 📋 Port tool execution tests to `code-rs/core/tests/suite/`
- 📋 Port unified executor tests (stdin session reuse, timeout handling)
- 📋 Run smoke tests and verify no regressions
2. **Phase 3 (Nov 1-15)**: TUI Rendering Tests
- 📋 Create `code-rs/tui/tests/` directory structure
- 📋 Port VT100 rendering tests (7+ test cases for wrapping/ANSI/emoji/CJK)
- 📋 Add golden file fixtures for visual regression testing
3. **Phase 4 (Nov 16-30)**: Final Cleanup
- 📋 Audit coverage comparison, confirm no orphaned tests
- 📋 Remove upstream test directories if appropriate
- 📋 Archive playbook to `docs/archive/`
**Current Test Scaffolding:**
- ✅ Added base scenarios in `tui/tests/ui_smoke.rs`
- ✅ Added `chatwidget::smoke_helpers` with `ChatWidgetHarness` and assertion helpers
- 📋 Extract similar helpers for `App` and executor contexts
- 📋 Strengthen assertions for exec/approval/tool flows
See `docs/migration/legacy-tests-retirement.md` for detailed checklist, risk assessment, and success criteria.
## Upstream Tracking Process
- **Monthly cadence**: First Monday = quick diff, Second Monday = merge planning (see `docs/maintenance/upstream-diff.md`).
- Use the scripts under `scripts/upstream-merge/` (`diff-crates.sh`, `highlight-critical-changes.sh`, `log-merge.sh`) as described in `docs/maintenance/upstream-diff.md`.
- Monitor `codex-rs/core/prompt.md`, executor/plan tool updates, and API schema changes during each upstream merge.
- Maintain a CHANGELOG section summarizing upstream features adopted vs intentionally skipped.
- For reusable crates we adopt, pin the upstream commit hash and bump deliberately (documenting any fork-specific patches).
## Execution Phases
1. **Baseline & Tooling** ✅ *Complete (2025-10-05)*
- ✅ Reverted/deleted unused compatibility modules (compat.rs, app_backtrack.rs, etc.)
- ✅ Installed upstream diff scripts; verified they produce baseline reports (`scripts/upstream-merge/`)
- ✅ Established parallel worktree workflow documentation
- ✅ Created comprehensive docs in `docs/migration/` and `docs/maintenance/`
2. **Crate Adoption Pass** ✅ *Complete (2025-10-05)*
- ✅ Adopted `codex-mcp-client` and `codex-responses-api-proxy` with thin wrappers
- ✅ Documented wrapper decisions and pinned versions in `UPSTREAM_PARITY_CHECKLIST.md`
- ✅ Verified build passes with upstream crate dependencies
3. **Fork Cleanup Pass** ✅ *Complete (2025-10-05)*
- ✅ Executed P0/P1 deletions from `DEAD_CODE_INVENTORY.md` (106 test files, ~35K lines)
- ✅ Archived outdated planning docs under `docs/archive/tui-migrations/`
- ✅ Removed `legacy_tests` feature flag and all associated test infrastructure
- 📋 Remaining: Audit `code-fork` feature usage and unused modules in `code-rs/core`
4. **Test Suite Rebuild** 🔄 *Phase 1 Complete, Phase 2-4 In Progress (Oct 20 - Nov 30)*
- ✅ Phase 1: Removed legacy test suites, verified smoke tests pass, build verification clean
- 📋 Phase 2 (Oct 20-31): Port 6 critical core runtime tests
- 📋 Phase 3 (Nov 1-15): Port 3 TUI rendering tests, add golden file fixtures
- 📋 Phase 4 (Nov 16-30): Final coverage audit and cleanup
- See `docs/migration/legacy-tests-retirement.md` for detailed plan
5. **Upstream Tracking & Maintenance** 🔄 *Ongoing*
- ✅ Established monthly cadence (First Monday: diff, Second Monday: merge planning)
- ✅ Created tracking scripts and logging infrastructure (`scripts/upstream-merge/`)
- 📋 Schedule first upstream diff review for next month
- 📋 Capture learnings from each parallel-agent run to refine workflow
## Next Actions (Priority Order)
### Completed (2025-10-05) ✅
1. ✅ **Audit remaining fork-specific code**
- ✅ Reviewed `code-fork` feature flag usage - properly used, gates 12 fork-specific TUI extensions
- ✅ Identified module usage in `code-rs/core` - all modules (`codex/`, `unified_exec/`, `exec_command/`) actively used
- ✅ Documented findings in `DEAD_CODE_INVENTORY.md` with comprehensive audit summary
2. ✅ **Document test scaffolding patterns**
- ✅ Documented ChatWidgetHarness and assertion helpers in `TEST_SUITE_RESET.md`
- ✅ Added usage examples, best practices, and extension guide
- ✅ Identified future test infrastructure needs (AppHarness, ExecutorHarness, MCPHarness)
### Active Work
1. **Begin Phase 2: Core Runtime Test Porting (Oct 20-31)**
- Port 6 critical test files from `codex-rs/core/tests/suite/` to `code-rs/core/tests/suite/`
- Focus: `model_tools.rs`, `tool_harness.rs`, `tools.rs`, `read_file.rs`, `view_image.rs`, `unified_exec.rs`
- See detailed checklist in `docs/migration/legacy-tests-retirement.md`
- **Blocker:** Requires `codex-rs/` directory (upstream checkout)
2. **Schedule upstream diff review**
- Run `scripts/upstream-merge/diff-crates.sh --all` on first Monday of month
- Review `CRITICAL-SUMMARY.md` for changes requiring attention
- Log decisions per `docs/maintenance/upstream-diff.md` workflow
- **Prerequisite:** Checkout upstream to `codex-rs/` directory
3. **Expand smoke test coverage** (ongoing)
- Strengthen assertions in `tui/tests/ui_smoke.rs` for exec/approval/tool flows
- Add executor and MCP integration test helpers following documented patterns
- Expand `smoke_helpers.rs` with additional assertion helpers as needed
## Success Metrics
Track progress against these deliverables:
| Metric | Baseline (2025-10-05) | Target | Status |
|--------|----------------------|--------|--------|
| **Codebase Size** | 35K lines removed | Clean tree, fork-only modules | ✅ Complete |
| **Test Files** | 106 legacy files deleted | 9 critical tests ported | 🔄 0/9 ported |
| **Test Coverage** | ~85% (estimated) | ≥90% with new suite | 📋 TBD |
| **Build Time** | 1m 30s (dev-fast) | ≤ baseline | ✅ No regression |
| **Upstream Crate Adoption** | 2/2 crates (mcp-client, responses-api-proxy) | Wrapper overhead minimal | ✅ Complete |
| **Documentation** | Migration docs created | Complete guides in docs/ | ✅ Complete |
| **Upstream Tracking** | Scripts installed | Monthly cadence active | 📋 First review pending |
## Deliverables
- ✅ Clean `code-rs` tree with only fork-relevant modules (Phase 1-3 complete)
- 🔄 Minimal but working test suite aligned with current behavior (Phase 2-4 in progress, 9 tests to port)
- ✅ Clearly documented integration points for upstream crates and prompts
- ✅ Migration playbooks and maintenance guides in `docs/migration/` and `docs/maintenance/`
- 📋 Monthly upstream tracking cadence established and operational
## Reference Docs
- `DEAD_CODE_INVENTORY.md` – current cleanup watchlist and outstanding audits.
- `TEST_SUITE_RESET.md` – smoke harness coverage and remaining porting tasks.
- `docs/maintenance/upstream-diff.md` – cadence, tooling, and logging workflow for upstream merges.
TEST_SUITE_RESET.md
+1 -1
View File
@@ -427,7 +427,7 @@ To add new test helpers:
- `MCPHarness` - Test harness for MCP server interactions (handshake, tool calls)
- VT100 replay tests - Capture/replay terminal sequences for rendering regression tests
See `docs/migration/tui-gap-report.md` for details on planned test automation.
Document future automation plans inline here as they evolve; remove stale pointers when work completes.
---
UPSTREAM_PARITY_CHECKLIST.md
+11 -201
View File
@@ -1,205 +1,15 @@
# Upstream Parity Maintenance Checklist
# Upstream Parity Quick Reference
Quick reference for maintaining parity with codex-rs MCP components.
**Status (October 6, 2025):** `code-ansi-escape`, `code-backend-client`, `code-cloud-tasks-client`, `code-execpolicy`, `code-git-apply`, and `code-linux-sandbox` all re-export the upstream `codex-*` crates. Thin wrappers for `code-mcp-client`, `code-mcp-types`, and `code-responses-api-proxy` remain in place and should stay minimal.
**Status (2025-10-06):** MCP crates re-export upstream `codex-rs` crates. Monitoring now focuses on upstream commits and wrapper glue.
## Cadence
- **First Monday:** run `scripts/upstream-merge/diff-crates.sh --all` and review `.github/auto/upstream-diffs/SUMMARY.md` for unexpected churn.
- **Second Monday (if needed):** run `scripts/upstream-merge/highlight-critical-changes.sh --all`, capture decisions via `log-merge.sh`, and update wrapper docs if APIs shift.
- **Ad hoc:** trigger the cadence immediately when upstream ships MCP/runtime changes we rely on.
## Monthly Cadence
## Spot Checks
- Ensure wrapper crates only contain configuration glue (buffer sizes, binary names) and continue to compile without warnings.
- When adopting new upstream releases, document any fork-only hooks inside the crate README or `docs/maintenance/upstream-diff.md`.
- Security-sensitive crates (`code-execpolicy`, `code-linux-sandbox`) should get manual smoke checks after each bump.
We follow a two-step cycle aligned with the broader upstream diff process (see `docs/maintenance/upstream-diff.md`).
### First Monday — Quick Diff
- [ ] Fetch latest upstream: `git fetch upstream`
- [ ] Inspect new commits: `git log HEAD..upstream/main --oneline`
- [ ] Run structural diff: `./scripts/upstream-merge/diff-crates.sh --all`
- [ ] Review summary: `cat .github/auto/upstream-diffs/SUMMARY.md`
- [ ] Check key crates for breaking changes:
- [ ] `mcp-client/`
- [ ] `responses-api-proxy/`
- [ ] `process-hardening/`
- [ ] `mcp-types/`
- [ ] Decide whether a merge plan is required this month
### Second Monday — Merge Planning (if needed)
- [ ] Highlight critical changes: `./scripts/upstream-merge/highlight-critical-changes.sh --all`
- [ ] Review critical summary: `cat .github/auto/upstream-diffs/critical-changes/CRITICAL-SUMMARY.md`
- [ ] Initialize merge log: `./scripts/upstream-merge/log-merge.sh init upstream/main`
- [ ] Categorize each change (adopt/adapt/preserve) and document decisions
- [ ] Identify wrapper updates required in `code-rs`
- [ ] Queue follow-up work items (smoke tests, documentation)
### Standing Checks
- [ ] Run `cargo build --workspace`
- [ ] Run `cargo test -p mcp-types`
- [ ] Review relevant upstream issues/PRs affecting MCP components
- [ ] Note manual validation required for MCP client/proxy flows (legacy integration suite removed)
## Monthly Deep Dive
### Performance
- [ ] Benchmark MCP client with large responses (1MB, 2MB, 5MB)
- [ ] Measure proxy latency and throughput
- [ ] Compare performance vs previous month baseline
- [ ] Document any regressions or improvements
### Security
- [ ] Run `cargo audit` on full dependency tree
- [ ] Verify process hardening on all platforms:
- [ ] Linux: `cat /proc/$(pgrep code-responses)/status | grep Dumpable`
- [ ] macOS: Attempt lldb attach (should fail)
- [ ] Windows: (TODO: Define verification)
- [ ] Review upstream security advisories
- [ ] Check for CVEs in MCP component dependencies
### Code Review (Re-Export Model)
- [ ] Review all upstream changes since last month
- [ ] Verify re-export wrappers (`code-mcp-client`, `code-responses-api-proxy`, `code-process-hardening`, `code-mcp-types`) remain thin/minimal
- [ ] Identify opportunities for code-rs usage patterns to simplify
- [ ] Identify code-rs integration improvements worth contributing upstream
- [ ] **Note:** Divergence now minimal; focus on ensuring re-exports stay up-to-date
### Documentation
- [ ] Update upstream-mcp-reuse-strategy.md with new findings
- [ ] Document any new workarounds or patches needed
- [ ] Review and update implementation guide if API changed
- [ ] Update this checklist with new monitoring needs
## Per Upstream Release
### Pre-Upgrade
- [ ] Read full changelog for breaking changes
- [ ] Identify deprecated APIs used by code-rs
- [ ] Plan migration for breaking changes
- [ ] Create test branch for upgrade
### Testing (Post-Phase 1 Cleanup)
- [ ] Run full build: `cargo build --workspace`
- [ ] Run minimal test suite: `cargo test -p mcp-types -p code-linux-sandbox -p code-cloud-tasks`
- [ ] Manual integration validation with real MCP servers (no automated tests currently)
- [ ] Security hardening validation (landlock tests in `linux-sandbox`)
- [ ] Cross-platform build verification (Linux, macOS, Windows)
- [ ] **Note:** Full integration test suite removed; rely on build verification and manual validation
### Documentation
- [ ] Update dependency versions in docs
- [ ] Document migration steps for breaking changes
- [ ] Update examples if API changed
- [ ] Add release notes to code-rs changelog
### Deployment
- [ ] Update Cargo.toml with new version pins
- [ ] Create PR with upgrade changes
- [ ] Run CI/CD pipeline
- [ ] Deploy to staging environment
- [ ] Monitor for issues post-deployment
## Critical Alerts (Immediate Action)
### Security Advisory
- [ ] Assess impact on code-rs
- [ ] Determine if affected components are used
- [ ] Create hotfix branch if needed
- [ ] Apply patches immediately
- [ ] Notify team of security update
- [ ] Deploy emergency update
### Upstream API Breakage
- [ ] Identify affected code in code-rs
- [ ] Assess migration effort
- [ ] Choose mitigation strategy:
- [ ] Pin to last working version (temporary)
- [ ] Implement adapter/wrapper
- [ ] Migrate to new API
- [ ] Fork and maintain (last resort)
- [ ] Create migration plan
- [ ] Execute migration or pin version
### Upstream Abandonment
- [ ] Assess recent activity level
- [ ] Contact upstream maintainers
- [ ] Evaluate fork maintenance cost
- [ ] Decision matrix:
- [ ] Wait and monitor (if temporary)
- [ ] Take over maintenance
- [ ] Fork permanently
- [ ] Replace with alternative
- [ ] Document decision and rationale
## Quick Commands Reference (Updated for Re-Export Model)
```bash
# Check upstream changes
cd codex-rs
git fetch origin
git log origin/main --since="1 week ago" -- mcp-client/ responses-api-proxy/ process-hardening/ mcp-types/
# Verify re-exports build with upstream changes
cd code-rs
cargo build --workspace
# Run minimal test suite
cargo test -p mcp-types
cargo test -p code-linux-sandbox
cargo test -p code-cloud-tasks
# Security audit
cargo audit
# Check process hardening (Linux)
ps aux | grep code-responses
cat /proc/$(pgrep code-responses)/status | grep Dumpable
# Check process hardening (macOS)
ps aux | grep code-responses
lldb -p $(pgrep code-responses) # Should fail
# Verify re-export wrappers are minimal (no local forks to diff)
# code-mcp-client, code-responses-api-proxy, code-process-hardening, code-mcp-types
# should all be thin re-export wrappers pointing to codex-rs counterparts
```
## GitHub Notifications Setup
1. Go to: https://github.com/openai/codex-rs
2. Click "Watch" → "Custom"
3. Enable notifications for:
- [x] Issues
- [x] Pull requests
- [x] Releases
- [x] Security alerts
4. Set up custom rules:
- Path filter: `mcp-client/**`
- Path filter: `responses-api-proxy/**`
- Path filter: `process-hardening/**`
- Path filter: `mcp-types/**`
## Metrics to Track (Updated for Re-Export Model)
| Metric | Target | Current (2025-10-05) | Trend |
|--------|--------|----------------------|-------|
| Upstream divergence (wrapper LOC) | <50 | ~0 (re-exports only) | ✅ |
| Re-export wrapper complexity | Minimal | Thin wrappers | ✅ |
| Build success with latest upstream | 100% | ✅ | — |
| Security audit issues | 0 | TBD | — |
| Days since upstream sync check | <7 | 0 | — |
**Note:** With re-export model, "divergence" is effectively zero. Focus shifts to monitoring upstream API stability and ensuring wrappers remain minimal.
## Contact
- **Upstream Issues**: https://github.com/openai/codex-rs/issues
- **Upstream Discussions**: https://github.com/openai/codex-rs/discussions
- **Security**: security@openai.com (for vulnerabilities)
## Archive
When upstream reuse is complete and stable, archive this checklist with:
- Final divergence report
- Lessons learned
- Recommendations for future component reuse
---
**Last Updated**: 2025-10-05
**Next Review**: 2025-10-12
**Model**: Re-export wrappers (`code-mcp-client`, `code-responses-api-proxy`, `code-process-hardening`, `code-mcp-types` all re-export from `codex-rs`)
For the full workflow, see `docs/maintenance/upstream-diff.md`.
code-rs/Cargo.lock
Generated
+792 -43
View File
File diff suppressed because it is too large Load Diff
code-rs/Cargo.toml
+6
View File
@@ -74,6 +74,12 @@ code-utils-json-to-toml = { path = "utils/json-to-toml" }
code-utils-readiness = { path = "utils/readiness" }
mcp-types = { path = "mcp-types", package = "code-mcp-types" }
codex-mcp-client = { path = "../codex-rs/mcp-client" }
codex-ansi-escape = { path = "../codex-rs/ansi-escape" }
codex-backend-client = { path = "../codex-rs/backend-client" }
codex-cloud-tasks-client = { path = "../codex-rs/cloud-tasks-client" }
codex-execpolicy = { path = "../codex-rs/execpolicy" }
codex-git-apply = { path = "../codex-rs/git-apply" }
codex-linux-sandbox = { path = "../codex-rs/linux-sandbox" }
codex-process-hardening = { path = "../codex-rs/process-hardening" }
codex-responses-api-proxy = { path = "../codex-rs/responses-api-proxy" }
upstream-mcp-types = { path = "../codex-rs/mcp-types", package = "mcp-types" }
code-rs/ansi-escape/Cargo.toml
+4 -6
View File
@@ -7,10 +7,8 @@ version = { workspace = true }
name = "code_ansi_escape"
path = "src/lib.rs"
[lints]
workspace = true
[dependencies]
ansi-to-tui = { workspace = true }
ratatui = { workspace = true, features = [
"unstable-rendered-line-info",
"unstable-widget-ref",
] }
tracing = { workspace = true, features = ["log"] }
codex-ansi-escape = { workspace = true }
code-rs/ansi-escape/README.md
-15
View File
@@ -1,15 +0,0 @@
# oai-codex-ansi-escape
Small helper functions that wrap functionality from
<https://crates.io/crates/ansi-to-tui>:
```rust
pub fn ansi_escape_line(s: &str) -> Line<'static>
pub fn ansi_escape<'a>(s: &'a str) -> Text<'a>
```
Advantages:
- `ansi_to_tui::IntoText` is not in scope for the entire TUI crate
- we `panic!()` and log if `IntoText` returns an `Err` and log it so that
the caller does not have to deal with it
code-rs/ansi-escape/src/lib.rs
+3 -39
View File
@@ -1,39 +1,3 @@
use ansi_to_tui::Error;
use ansi_to_tui::IntoText;
use ratatui::text::Line;
use ratatui::text::Text;
/// This function should be used when the contents of `s` are expected to match
/// a single line. If multiple lines are found, a warning is logged and only the
/// first line is returned.
pub fn ansi_escape_line(s: &str) -> Line<'static> {
let text = ansi_escape(s);
match text.lines.as_slice() {
[] => "".into(),
[only] => only.clone(),
[first, rest @ ..] => {
tracing::warn!("ansi_escape_line: expected a single line, got {first:?} and {rest:?}");
first.clone()
}
}
}
pub fn ansi_escape(s: &str) -> Text<'static> {
// to_text() claims to be faster, but introduces complex lifetime issues
// such that it's not worth it.
match s.into_text() {
Ok(text) => text,
Err(err) => match err {
Error::NomError(message) => {
tracing::error!(
"ansi_to_tui NomError docs claim should never happen when parsing `{s}`: {message}"
);
panic!();
}
Error::Utf8Error(utf8error) => {
tracing::error!("Utf8Error: {utf8error}");
panic!();
}
},
}
}
//! Thin wrapper around the upstream `codex-ansi-escape` crate so existing
//! `code-ansi-escape` imports continue to work while we track upstream updates.
pub use codex_ansi_escape::*;
code-rs/backend-client/Cargo.toml
+4 -8
View File
@@ -7,12 +7,8 @@ publish = false
[lib]
path = "src/lib.rs"
[dependencies]
anyhow = "1"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }
code-backend-openapi-models = { path = "../code-backend-openapi-models" }
[lints]
workspace = true
[dev-dependencies]
pretty_assertions = "1"
[dependencies]
codex-backend-client = { workspace = true }
code-rs/backend-client/src/client.rs
-244
View File
@@ -1,244 +0,0 @@
use crate::types::CodeTaskDetailsResponse;
use crate::types::PaginatedListTaskListItem;
use crate::types::TurnAttemptsSiblingTurnsResponse;
use anyhow::Result;
use reqwest::header::AUTHORIZATION;
use reqwest::header::CONTENT_TYPE;
use reqwest::header::HeaderMap;
use reqwest::header::HeaderName;
use reqwest::header::HeaderValue;
use reqwest::header::USER_AGENT;
use serde::de::DeserializeOwned;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum PathStyle {
/// /api/codex/…
CodexApi,
/// /wham/…
ChatGptApi,
}
impl PathStyle {
pub fn from_base_url(base_url: &str) -> Self {
if base_url.contains("/backend-api") {
PathStyle::ChatGptApi
} else {
PathStyle::CodexApi
}
}
}
#[derive(Clone, Debug)]
pub struct Client {
base_url: String,
http: reqwest::Client,
bearer_token: Option<String>,
user_agent: Option<HeaderValue>,
chatgpt_account_id: Option<String>,
path_style: PathStyle,
}
impl Client {
pub fn new(base_url: impl Into<String>) -> Result<Self> {
let mut base_url = base_url.into();
// Normalize common ChatGPT hostnames to include /backend-api so we hit the WHAM paths.
// Also trim trailing slashes for consistent URL building.
while base_url.ends_with('/') {
base_url.pop();
}
if (base_url.starts_with("https://chatgpt.com")
|| base_url.starts_with("https://chat.openai.com"))
&& !base_url.contains("/backend-api")
{
base_url = format!("{base_url}/backend-api");
}
let http = reqwest::Client::builder().build()?;
let path_style = PathStyle::from_base_url(&base_url);
Ok(Self {
base_url,
http,
bearer_token: None,
user_agent: None,
chatgpt_account_id: None,
path_style,
})
}
pub fn with_bearer_token(mut self, token: impl Into<String>) -> Self {
self.bearer_token = Some(token.into());
self
}
pub fn with_user_agent(mut self, ua: impl Into<String>) -> Self {
if let Ok(hv) = HeaderValue::from_str(&ua.into()) {
self.user_agent = Some(hv);
}
self
}
pub fn with_chatgpt_account_id(mut self, account_id: impl Into<String>) -> Self {
self.chatgpt_account_id = Some(account_id.into());
self
}
pub fn with_path_style(mut self, style: PathStyle) -> Self {
self.path_style = style;
self
}
fn headers(&self) -> HeaderMap {
let mut h = HeaderMap::new();
if let Some(ua) = &self.user_agent {
h.insert(USER_AGENT, ua.clone());
} else {
h.insert(USER_AGENT, HeaderValue::from_static("codex-cli"));
}
if let Some(token) = &self.bearer_token {
let value = format!("Bearer {token}");
if let Ok(hv) = HeaderValue::from_str(&value) {
h.insert(AUTHORIZATION, hv);
}
}
if let Some(acc) = &self.chatgpt_account_id
&& let Ok(name) = HeaderName::from_bytes(b"ChatGPT-Account-Id")
&& let Ok(hv) = HeaderValue::from_str(acc)
{
h.insert(name, hv);
}
h
}
async fn exec_request(
&self,
req: reqwest::RequestBuilder,
method: &str,
url: &str,
) -> Result<(String, String)> {
let res = req.send().await?;
let status = res.status();
let ct = res
.headers()
.get(CONTENT_TYPE)
.and_then(|v| v.to_str().ok())
.unwrap_or("")
.to_string();
let body = res.text().await.unwrap_or_default();
if !status.is_success() {
anyhow::bail!("{method} {url} failed: {status}; content-type={ct}; body={body}");
}
Ok((body, ct))
}
fn decode_json<T: DeserializeOwned>(&self, url: &str, ct: &str, body: &str) -> Result<T> {
match serde_json::from_str::<T>(body) {
Ok(v) => Ok(v),
Err(e) => {
anyhow::bail!("Decode error for {url}: {e}; content-type={ct}; body={body}");
}
}
}
pub async fn list_tasks(
&self,
limit: Option<i32>,
task_filter: Option<&str>,
environment_id: Option<&str>,
) -> Result<PaginatedListTaskListItem> {
let url = match self.path_style {
PathStyle::CodexApi => format!("{}/api/codex/tasks/list", self.base_url),
PathStyle::ChatGptApi => format!("{}/wham/tasks/list", self.base_url),
};
let req = self.http.get(&url).headers(self.headers());
let req = if let Some(lim) = limit {
req.query(&[("limit", lim)])
} else {
req
};
let req = if let Some(tf) = task_filter {
req.query(&[("task_filter", tf)])
} else {
req
};
let req = if let Some(id) = environment_id {
req.query(&[("environment_id", id)])
} else {
req
};
let (body, ct) = self.exec_request(req, "GET", &url).await?;
self.decode_json::<PaginatedListTaskListItem>(&url, &ct, &body)
}
pub async fn get_task_details(&self, task_id: &str) -> Result<CodeTaskDetailsResponse> {
let (parsed, _body, _ct) = self.get_task_details_with_body(task_id).await?;
Ok(parsed)
}
pub async fn get_task_details_with_body(
&self,
task_id: &str,
) -> Result<(CodeTaskDetailsResponse, String, String)> {
let url = match self.path_style {
PathStyle::CodexApi => format!("{}/api/codex/tasks/{}", self.base_url, task_id),
PathStyle::ChatGptApi => format!("{}/wham/tasks/{}", self.base_url, task_id),
};
let req = self.http.get(&url).headers(self.headers());
let (body, ct) = self.exec_request(req, "GET", &url).await?;
let parsed: CodeTaskDetailsResponse = self.decode_json(&url, &ct, &body)?;
Ok((parsed, body, ct))
}
pub async fn list_sibling_turns(
&self,
task_id: &str,
turn_id: &str,
) -> Result<TurnAttemptsSiblingTurnsResponse> {
let url = match self.path_style {
PathStyle::CodexApi => format!(
"{}/api/codex/tasks/{}/turns/{}/sibling_turns",
self.base_url, task_id, turn_id
),
PathStyle::ChatGptApi => format!(
"{}/wham/tasks/{}/turns/{}/sibling_turns",
self.base_url, task_id, turn_id
),
};
let req = self.http.get(&url).headers(self.headers());
let (body, ct) = self.exec_request(req, "GET", &url).await?;
self.decode_json::<TurnAttemptsSiblingTurnsResponse>(&url, &ct, &body)
}
/// Create a new task (user turn) by POSTing to the appropriate backend path
/// based on `path_style`. Returns the created task id.
pub async fn create_task(&self, request_body: serde_json::Value) -> Result<String> {
let url = match self.path_style {
PathStyle::CodexApi => format!("{}/api/codex/tasks", self.base_url),
PathStyle::ChatGptApi => format!("{}/wham/tasks", self.base_url),
};
let req = self
.http
.post(&url)
.headers(self.headers())
.header(CONTENT_TYPE, HeaderValue::from_static("application/json"))
.json(&request_body);
let (body, ct) = self.exec_request(req, "POST", &url).await?;
// Extract id from JSON: prefer `task.id`; fallback to top-level `id` when present.
match serde_json::from_str::<serde_json::Value>(&body) {
Ok(v) => {
if let Some(id) = v
.get("task")
.and_then(|t| t.get("id"))
.and_then(|s| s.as_str())
{
Ok(id.to_string())
} else if let Some(id) = v.get("id").and_then(|s| s.as_str()) {
Ok(id.to_string())
} else {
anyhow::bail!(
"POST {url} succeeded but no task id found; content-type={ct}; body={body}"
);
}
}
Err(e) => anyhow::bail!("Decode error for {url}: {e}; content-type={ct}; body={body}"),
}
}
}
code-rs/backend-client/src/lib.rs
+2 -9
View File
@@ -1,9 +1,2 @@
mod client;
pub mod types;
pub use client::Client;
pub use types::CodeTaskDetailsResponse;
pub use types::CodeTaskDetailsResponseExt;
pub use types::PaginatedListTaskListItem;
pub use types::TaskListItem;
pub use types::TurnAttemptsSiblingTurnsResponse;
//! Thin wrapper around the upstream `codex-backend-client` crate.
pub use codex_backend_client::*;
code-rs/backend-client/src/types.rs
-369
View File
@@ -1,369 +0,0 @@
pub use code_backend_openapi_models::models::PaginatedListTaskListItem;
pub use code_backend_openapi_models::models::TaskListItem;
use serde::Deserialize;
use serde::de::Deserializer;
use serde_json::Value;
use std::collections::HashMap;
/// Hand-rolled models for the Cloud Tasks task-details response.
/// The generated OpenAPI models are pretty bad. This is a half-step
/// towards hand-rolling them.
#[derive(Clone, Debug, Deserialize)]
pub struct CodeTaskDetailsResponse {
#[serde(default)]
pub current_user_turn: Option<Turn>,
#[serde(default)]
pub current_assistant_turn: Option<Turn>,
#[serde(default)]
pub current_diff_task_turn: Option<Turn>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct Turn {
#[serde(default)]
pub id: Option<String>,
#[serde(default)]
pub attempt_placement: Option<i64>,
#[serde(default, rename = "turn_status")]
pub turn_status: Option<String>,
#[serde(default, deserialize_with = "deserialize_vec")]
pub sibling_turn_ids: Vec<String>,
#[serde(default, deserialize_with = "deserialize_vec")]
pub input_items: Vec<TurnItem>,
#[serde(default, deserialize_with = "deserialize_vec")]
pub output_items: Vec<TurnItem>,
#[serde(default)]
pub worklog: Option<Worklog>,
#[serde(default)]
pub error: Option<TurnError>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct TurnItem {
#[serde(rename = "type", default)]
pub kind: String,
#[serde(default)]
pub role: Option<String>,
#[serde(default, deserialize_with = "deserialize_vec")]
pub content: Vec<ContentFragment>,
#[serde(default)]
pub diff: Option<String>,
#[serde(default)]
pub output_diff: Option<DiffPayload>,
}
#[derive(Clone, Debug, Deserialize)]
#[serde(untagged)]
pub enum ContentFragment {
Structured(StructuredContent),
Text(String),
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct StructuredContent {
#[serde(rename = "content_type", default)]
pub content_type: Option<String>,
#[serde(default)]
pub text: Option<String>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct DiffPayload {
#[serde(default)]
pub diff: Option<String>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct Worklog {
#[serde(default, deserialize_with = "deserialize_vec")]
pub messages: Vec<WorklogMessage>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct WorklogMessage {
#[serde(default)]
pub author: Option<Author>,
#[serde(default)]
pub content: Option<WorklogContent>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct Author {
#[serde(default)]
pub role: Option<String>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct WorklogContent {
#[serde(default)]
pub parts: Vec<ContentFragment>,
}
#[derive(Clone, Debug, Default, Deserialize)]
pub struct TurnError {
#[serde(default)]
pub code: Option<String>,
#[serde(default)]
pub message: Option<String>,
}
impl ContentFragment {
fn text(&self) -> Option<&str> {
match self {
ContentFragment::Structured(inner) => {
if inner
.content_type
.as_deref()
.map(|ct| ct.eq_ignore_ascii_case("text"))
.unwrap_or(false)
{
inner.text.as_deref().filter(|s| !s.is_empty())
} else {
None
}
}
ContentFragment::Text(raw) => {
if raw.trim().is_empty() {
None
} else {
Some(raw.as_str())
}
}
}
}
}
impl TurnItem {
fn text_values(&self) -> Vec<String> {
self.content
.iter()
.filter_map(|fragment| fragment.text().map(str::to_string))
.collect()
}
fn diff_text(&self) -> Option<String> {
if self.kind == "output_diff" {
if let Some(diff) = &self.diff
&& !diff.is_empty()
{
return Some(diff.clone());
}
} else if self.kind == "pr"
&& let Some(payload) = &self.output_diff
&& let Some(diff) = &payload.diff
&& !diff.is_empty()
{
return Some(diff.clone());
}
None
}
}
impl Turn {
fn unified_diff(&self) -> Option<String> {
self.output_items.iter().find_map(TurnItem::diff_text)
}
fn message_texts(&self) -> Vec<String> {
let mut out: Vec<String> = self
.output_items
.iter()
.filter(|item| item.kind == "message")
.flat_map(TurnItem::text_values)
.collect();
if let Some(log) = &self.worklog {
for message in &log.messages {
if message.is_assistant() {
out.extend(message.text_values());
}
}
}
out
}
fn user_prompt(&self) -> Option<String> {
let parts: Vec<String> = self
.input_items
.iter()
.filter(|item| item.kind == "message")
.filter(|item| {
item.role
.as_deref()
.map(|r| r.eq_ignore_ascii_case("user"))
.unwrap_or(true)
})
.flat_map(TurnItem::text_values)
.collect();
if parts.is_empty() {
None
} else {
Some(parts.join(
"
",
))
}
}
fn error_summary(&self) -> Option<String> {
self.error.as_ref().and_then(TurnError::summary)
}
}
impl WorklogMessage {
fn is_assistant(&self) -> bool {
self.author
.as_ref()
.and_then(|a| a.role.as_deref())
.map(|role| role.eq_ignore_ascii_case("assistant"))
.unwrap_or(false)
}
fn text_values(&self) -> Vec<String> {
self.content
.as_ref()
.map(|content| {
content
.parts
.iter()
.filter_map(|fragment| fragment.text().map(str::to_string))
.collect()
})
.unwrap_or_default()
}
}
impl TurnError {
fn summary(&self) -> Option<String> {
let code = self.code.as_deref().unwrap_or("");
let message = self.message.as_deref().unwrap_or("");
match (code.is_empty(), message.is_empty()) {
(true, true) => None,
(false, true) => Some(code.to_string()),
(true, false) => Some(message.to_string()),
(false, false) => Some(format!("{code}: {message}")),
}
}
}
pub trait CodeTaskDetailsResponseExt {
/// Attempt to extract a unified diff string from the assistant or diff turn.
fn unified_diff(&self) -> Option<String>;
/// Extract assistant text output messages (no diff) from current turns.
fn assistant_text_messages(&self) -> Vec<String>;
/// Extract the user's prompt text from the current user turn, when present.
fn user_text_prompt(&self) -> Option<String>;
/// Extract an assistant error message (if the turn failed and provided one).
fn assistant_error_message(&self) -> Option<String>;
}
impl CodeTaskDetailsResponseExt for CodeTaskDetailsResponse {
fn unified_diff(&self) -> Option<String> {
[
self.current_diff_task_turn.as_ref(),
self.current_assistant_turn.as_ref(),
]
.into_iter()
.flatten()
.find_map(Turn::unified_diff)
}
fn assistant_text_messages(&self) -> Vec<String> {
let mut out = Vec::new();
for turn in [
self.current_diff_task_turn.as_ref(),
self.current_assistant_turn.as_ref(),
]
.into_iter()
.flatten()
{
out.extend(turn.message_texts());
}
out
}
fn user_text_prompt(&self) -> Option<String> {
self.current_user_turn.as_ref().and_then(Turn::user_prompt)
}
fn assistant_error_message(&self) -> Option<String> {
self.current_assistant_turn
.as_ref()
.and_then(Turn::error_summary)
}
}
fn deserialize_vec<'de, D, T>(deserializer: D) -> Result<Vec<T>, D::Error>
where
D: Deserializer<'de>,
T: Deserialize<'de>,
{
Option::<Vec<T>>::deserialize(deserializer).map(|opt| opt.unwrap_or_default())
}
#[derive(Clone, Debug, Deserialize)]
pub struct TurnAttemptsSiblingTurnsResponse {
#[serde(default)]
pub sibling_turns: Vec<HashMap<String, Value>>,
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
fn fixture(name: &str) -> CodeTaskDetailsResponse {
let json = match name {
"diff" => include_str!("../tests/fixtures/task_details_with_diff.json"),
"error" => include_str!("../tests/fixtures/task_details_with_error.json"),
other => panic!("unknown fixture {other}"),
};
serde_json::from_str(json).expect("fixture should deserialize")
}
#[test]
fn unified_diff_prefers_current_diff_task_turn() {
let details = fixture("diff");
let diff = details.unified_diff().expect("diff present");
assert!(diff.contains("diff --git"));
}
#[test]
fn unified_diff_falls_back_to_pr_output_diff() {
let details = fixture("error");
let diff = details.unified_diff().expect("diff from pr output");
assert!(diff.contains("lib.rs"));
}
#[test]
fn assistant_text_messages_extracts_text_content() {
let details = fixture("diff");
let messages = details.assistant_text_messages();
assert_eq!(messages, vec!["Assistant response".to_string()]);
}
#[test]
fn user_text_prompt_joins_parts_with_spacing() {
let details = fixture("diff");
let prompt = details.user_text_prompt().expect("prompt present");
assert_eq!(
prompt,
"First line
Second line"
);
}
#[test]
fn assistant_error_message_combines_code_and_message() {
let details = fixture("error");
let msg = details
.assistant_error_message()
.expect("error should be present");
assert_eq!(msg, "APPLY_FAILED: Patch could not be applied");
}
}
code-rs/backend-client/tests/fixtures/task_details_with_diff.json
Vendored
-38
View File
@@ -1,38 +0,0 @@
{
"task": {
"id": "task_123",
"title": "Refactor cloud task client",
"archived": false,
"external_pull_requests": []
},
"current_user_turn": {
"input_items": [
{
"type": "message",
"role": "user",
"content": [
{ "content_type": "text", "text": "First line" },
{ "content_type": "text", "text": "Second line" }
]
}
]
},
"current_assistant_turn": {
"output_items": [
{
"type": "message",
"content": [
{ "content_type": "text", "text": "Assistant response" }
]
}
]
},
"current_diff_task_turn": {
"output_items": [
{
"type": "output_diff",
"diff": "diff --git a/src/main.rs b/src/main.rs\n+fn main() { println!(\"hi\"); }\n"
}
]
}
}
code-rs/backend-client/tests/fixtures/task_details_with_error.json
Vendored
-22
View File
@@ -1,22 +0,0 @@
{
"task": {
"id": "task_456",
"title": "Investigate failure",
"archived": false,
"external_pull_requests": []
},
"current_assistant_turn": {
"output_items": [
{
"type": "pr",
"output_diff": {
"diff": "diff --git a/lib.rs b/lib.rs\n+pub fn hello() {}\n"
}
}
],
"error": {
"code": "APPLY_FAILED",
"message": "Patch could not be applied"
}
}
}
code-rs/cloud-tasks-client/Cargo.toml
+3 -11
View File
@@ -12,16 +12,8 @@ workspace = true
[features]
default = ["online"]
online = ["dep:code-backend-client"]
mock = []
online = ["codex-cloud-tasks-client/online"]
mock = ["codex-cloud-tasks-client/mock"]
[dependencies]
anyhow = "1"
async-trait = "0.1"
chrono = { version = "0.4", features = ["serde"] }
diffy = "0.4.2"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
thiserror = "2.0.12"
code-backend-client = { path = "../backend-client", optional = true }
code-git-apply = { path = "../git-apply" }
codex-cloud-tasks-client = { workspace = true }
code-rs/cloud-tasks-client/src/api.rs
-158
View File
@@ -1,158 +0,0 @@
use chrono::DateTime;
use chrono::Utc;
use serde::Deserialize;
use serde::Serialize;
pub type Result<T> = std::result::Result<T, CloudTaskError>;
#[derive(Debug, thiserror::Error)]
pub enum CloudTaskError {
#[error("unimplemented: {0}")]
Unimplemented(&'static str),
#[error("http error: {0}")]
Http(String),
#[error("io error: {0}")]
Io(String),
#[error("{0}")]
Msg(String),
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(transparent)]
pub struct TaskId(pub String);
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum TaskStatus {
Pending,
Ready,
Applied,
Error,
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct TaskSummary {
pub id: TaskId,
pub title: String,
pub status: TaskStatus,
pub updated_at: DateTime<Utc>,
/// Backend environment identifier (when available)
pub environment_id: Option<String>,
/// Human-friendly environment label (when available)
pub environment_label: Option<String>,
pub summary: DiffSummary,
/// True when the backend reports this task as a code review.
#[serde(default)]
pub is_review: bool,
/// Number of assistant attempts (best-of-N), when reported by the backend.
#[serde(default)]
pub attempt_total: Option<usize>,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)]
pub enum AttemptStatus {
Pending,
InProgress,
Completed,
Failed,
Cancelled,
#[default]
Unknown,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct TurnAttempt {
pub turn_id: String,
pub attempt_placement: Option<i64>,
pub created_at: Option<DateTime<Utc>>,
pub status: AttemptStatus,
pub diff: Option<String>,
pub messages: Vec<String>,
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum ApplyStatus {
Success,
Partial,
Error,
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct ApplyOutcome {
pub applied: bool,
pub status: ApplyStatus,
pub message: String,
#[serde(default)]
pub skipped_paths: Vec<String>,
#[serde(default)]
pub conflict_paths: Vec<String>,
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct CreatedTask {
pub id: TaskId,
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
pub struct DiffSummary {
pub files_changed: usize,
pub lines_added: usize,
pub lines_removed: usize,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct TaskText {
pub prompt: Option<String>,
pub messages: Vec<String>,
pub turn_id: Option<String>,
pub sibling_turn_ids: Vec<String>,
pub attempt_placement: Option<i64>,
pub attempt_status: AttemptStatus,
}
impl Default for TaskText {
fn default() -> Self {
Self {
prompt: None,
messages: Vec::new(),
turn_id: None,
sibling_turn_ids: Vec::new(),
attempt_placement: None,
attempt_status: AttemptStatus::Unknown,
}
}
}
#[async_trait::async_trait]
pub trait CloudBackend: Send + Sync {
async fn list_tasks(&self, env: Option<&str>) -> Result<Vec<TaskSummary>>;
async fn get_task_diff(&self, id: TaskId) -> Result<Option<String>>;
/// Return assistant output messages (no diff) when available.
async fn get_task_messages(&self, id: TaskId) -> Result<Vec<String>>;
/// Return the creating prompt and assistant messages (when available).
async fn get_task_text(&self, id: TaskId) -> Result<TaskText>;
/// Return any sibling attempts (best-of-N) for the given assistant turn.
async fn list_sibling_attempts(
&self,
task: TaskId,
turn_id: String,
) -> Result<Vec<TurnAttempt>>;
/// Dry-run apply (preflight) that validates whether the patch would apply cleanly.
/// Never modifies the working tree. When `diff_override` is supplied, the provided diff is
/// used instead of re-fetching the task details so callers can apply alternate attempts.
async fn apply_task_preflight(
&self,
id: TaskId,
diff_override: Option<String>,
) -> Result<ApplyOutcome>;
async fn apply_task(&self, id: TaskId, diff_override: Option<String>) -> Result<ApplyOutcome>;
async fn create_task(
&self,
env_id: &str,
prompt: &str,
git_ref: &str,
qa_mode: bool,
best_of_n: usize,
) -> Result<CreatedTask>;
}
code-rs/cloud-tasks-client/src/http.rs
-769
View File
@@ -1,769 +0,0 @@
use crate::ApplyOutcome;
use crate::ApplyStatus;
use crate::AttemptStatus;
use crate::CloudBackend;
use crate::CloudTaskError;
use crate::DiffSummary;
use crate::Result;
use crate::TaskId;
use crate::TaskStatus;
use crate::TaskSummary;
use crate::TurnAttempt;
use crate::api::TaskText;
use chrono::DateTime;
use chrono::Utc;
use code_backend_client as backend;
use code_backend_client::CodeTaskDetailsResponseExt;
#[derive(Clone)]
pub struct HttpClient {
pub base_url: String,
backend: backend::Client,
}
impl HttpClient {
pub fn new(base_url: impl Into<String>) -> anyhow::Result<Self> {
let base_url = base_url.into();
let backend = backend::Client::new(base_url.clone())?;
Ok(Self { base_url, backend })
}
pub fn with_bearer_token(mut self, token: impl Into<String>) -> Self {
self.backend = self.backend.clone().with_bearer_token(token);
self
}
pub fn with_user_agent(mut self, ua: impl Into<String>) -> Self {
self.backend = self.backend.clone().with_user_agent(ua);
self
}
pub fn with_chatgpt_account_id(mut self, account_id: impl Into<String>) -> Self {
self.backend = self.backend.clone().with_chatgpt_account_id(account_id);
self
}
fn tasks_api(&self) -> api::Tasks<'_> {
api::Tasks::new(self)
}
fn attempts_api(&self) -> api::Attempts<'_> {
api::Attempts::new(self)
}
fn apply_api(&self) -> api::Apply<'_> {
api::Apply::new(self)
}
}
#[async_trait::async_trait]
impl CloudBackend for HttpClient {
async fn list_tasks(&self, env: Option<&str>) -> Result<Vec<TaskSummary>> {
self.tasks_api().list(env).await
}
async fn get_task_diff(&self, id: TaskId) -> Result<Option<String>> {
self.tasks_api().diff(id).await
}
async fn get_task_messages(&self, id: TaskId) -> Result<Vec<String>> {
self.tasks_api().messages(id).await
}
async fn get_task_text(&self, id: TaskId) -> Result<TaskText> {
self.tasks_api().task_text(id).await
}
async fn list_sibling_attempts(
&self,
task: TaskId,
turn_id: String,
) -> Result<Vec<TurnAttempt>> {
self.attempts_api().list(task, turn_id).await
}
async fn apply_task(&self, id: TaskId, diff_override: Option<String>) -> Result<ApplyOutcome> {
self.apply_api().run(id, diff_override, false).await
}
async fn apply_task_preflight(
&self,
id: TaskId,
diff_override: Option<String>,
) -> Result<ApplyOutcome> {
self.apply_api().run(id, diff_override, true).await
}
async fn create_task(
&self,
env_id: &str,
prompt: &str,
git_ref: &str,
qa_mode: bool,
best_of_n: usize,
) -> Result<crate::CreatedTask> {
self.tasks_api()
.create(env_id, prompt, git_ref, qa_mode, best_of_n)
.await
}
}
mod api {
use super::*;
use serde_json::Value;
use std::cmp::Ordering;
use std::collections::HashMap;
pub(crate) struct Tasks<'a> {
base_url: &'a str,
backend: &'a backend::Client,
}
impl<'a> Tasks<'a> {
pub(crate) fn new(client: &'a HttpClient) -> Self {
Self {
base_url: &client.base_url,
backend: &client.backend,
}
}
pub(crate) async fn list(&self, env: Option<&str>) -> Result<Vec<TaskSummary>> {
let resp = self
.backend
.list_tasks(Some(20), Some("current"), env)
.await
.map_err(|e| CloudTaskError::Http(format!("list_tasks failed: {e}")))?;
let tasks: Vec<TaskSummary> = resp
.items
.into_iter()
.map(map_task_list_item_to_summary)
.collect();
append_error_log(&format!(
"http.list_tasks: env={} items={}",
env.unwrap_or("<all>"),
tasks.len()
));
Ok(tasks)
}
pub(crate) async fn diff(&self, id: TaskId) -> Result<Option<String>> {
let (details, body, ct) = self
.details_with_body(&id.0)
.await
.map_err(|e| CloudTaskError::Http(format!("get_task_details failed: {e}")))?;
if let Some(diff) = details.unified_diff() {
return Ok(Some(diff));
}
let _ = (body, ct);
Ok(None)
}
pub(crate) async fn messages(&self, id: TaskId) -> Result<Vec<String>> {
let (details, body, ct) = self
.details_with_body(&id.0)
.await
.map_err(|e| CloudTaskError::Http(format!("get_task_details failed: {e}")))?;
let mut msgs = details.assistant_text_messages();
if msgs.is_empty() {
msgs.extend(extract_assistant_messages_from_body(&body));
}
if !msgs.is_empty() {
return Ok(msgs);
}
if let Some(err) = details.assistant_error_message() {
return Ok(vec![format!("Task failed: {err}")]);
}
let url = match details_path(self.base_url, &id.0) {
Some(url) => url,
None => format!("{}/api/codex/tasks/{}", self.base_url, id.0),
};
Err(CloudTaskError::Http(format!(
"No assistant text messages in response. GET {url}; content-type={ct}; body={body}"
)))
}
pub(crate) async fn task_text(&self, id: TaskId) -> Result<TaskText> {
let (details, body, _ct) = self
.details_with_body(&id.0)
.await
.map_err(|e| CloudTaskError::Http(format!("get_task_details failed: {e}")))?;
let prompt = details.user_text_prompt();
let mut messages = details.assistant_text_messages();
if messages.is_empty() {
messages.extend(extract_assistant_messages_from_body(&body));
}
let assistant_turn = details.current_assistant_turn.as_ref();
let turn_id = assistant_turn.and_then(|turn| turn.id.clone());
let sibling_turn_ids = assistant_turn
.map(|turn| turn.sibling_turn_ids.clone())
.unwrap_or_default();
let attempt_placement = assistant_turn.and_then(|turn| turn.attempt_placement);
let attempt_status = attempt_status_from_str(
assistant_turn.and_then(|turn| turn.turn_status.as_deref()),
);
Ok(TaskText {
prompt,
messages,
turn_id,
sibling_turn_ids,
attempt_placement,
attempt_status,
})
}
pub(crate) async fn create(
&self,
env_id: &str,
prompt: &str,
git_ref: &str,
qa_mode: bool,
best_of_n: usize,
) -> Result<crate::CreatedTask> {
let mut input_items: Vec<serde_json::Value> = Vec::new();
input_items.push(serde_json::json!({
"type": "message",
"role": "user",
"content": [{ "content_type": "text", "text": prompt }]
}));
if let Ok(diff) = std::env::var("CODEX_STARTING_DIFF")
&& !diff.is_empty()
{
input_items.push(serde_json::json!({
"type": "pre_apply_patch",
"output_diff": { "diff": diff }
}));
}
let mut request_body = serde_json::json!({
"new_task": {
"environment_id": env_id,
"branch": git_ref,
"run_environment_in_qa_mode": qa_mode,
},
"input_items": input_items,
});
if best_of_n > 1
&& let Some(obj) = request_body.as_object_mut()
{
obj.insert(
"metadata".to_string(),
serde_json::json!({ "best_of_n": best_of_n }),
);
}
match self.backend.create_task(request_body).await {
Ok(id) => {
append_error_log(&format!(
"new_task: created id={id} env={} prompt_chars={}",
env_id,
prompt.chars().count()
));
Ok(crate::CreatedTask { id: TaskId(id) })
}
Err(e) => {
append_error_log(&format!(
"new_task: create failed env={} prompt_chars={}: {}",
env_id,
prompt.chars().count(),
e
));
Err(CloudTaskError::Http(format!("create_task failed: {e}")))
}
}
}
async fn details_with_body(
&self,
id: &str,
) -> anyhow::Result<(backend::CodeTaskDetailsResponse, String, String)> {
let (parsed, body, ct) = self.backend.get_task_details_with_body(id).await?;
Ok((parsed, body, ct))
}
}
pub(crate) struct Attempts<'a> {
backend: &'a backend::Client,
}
impl<'a> Attempts<'a> {
pub(crate) fn new(client: &'a HttpClient) -> Self {
Self {
backend: &client.backend,
}
}
pub(crate) async fn list(&self, task: TaskId, turn_id: String) -> Result<Vec<TurnAttempt>> {
let resp = self
.backend
.list_sibling_turns(&task.0, &turn_id)
.await
.map_err(|e| CloudTaskError::Http(format!("list_sibling_turns failed: {e}")))?;
let mut attempts: Vec<TurnAttempt> = resp
.sibling_turns
.iter()
.filter_map(turn_attempt_from_map)
.collect();
attempts.sort_by(compare_attempts);
Ok(attempts)
}
}
pub(crate) struct Apply<'a> {
backend: &'a backend::Client,
}
impl<'a> Apply<'a> {
pub(crate) fn new(client: &'a HttpClient) -> Self {
Self {
backend: &client.backend,
}
}
pub(crate) async fn run(
&self,
task_id: TaskId,
diff_override: Option<String>,
preflight: bool,
) -> Result<ApplyOutcome> {
let id = task_id.0.clone();
let diff = match diff_override {
Some(diff) => diff,
None => {
let details = self.backend.get_task_details(&id).await.map_err(|e| {
CloudTaskError::Http(format!("get_task_details failed: {e}"))
})?;
details.unified_diff().ok_or_else(|| {
CloudTaskError::Msg(format!("No diff available for task {id}"))
})?
}
};
if !is_unified_diff(&diff) {
let summary = summarize_patch_for_logging(&diff);
let mode = if preflight { "preflight" } else { "apply" };
append_error_log(&format!(
"apply_error: id={id} mode={mode} format=non-unified; {summary}"
));
return Ok(ApplyOutcome {
applied: false,
status: ApplyStatus::Error,
message: "Expected unified git diff; backend returned an incompatible format."
.to_string(),
skipped_paths: Vec::new(),
conflict_paths: Vec::new(),
});
}
let req = code_git_apply::ApplyGitRequest {
cwd: std::env::current_dir().unwrap_or_else(|_| std::env::temp_dir()),
diff: diff.clone(),
revert: false,
preflight,
};
let r = code_git_apply::apply_git_patch(&req)
.map_err(|e| CloudTaskError::Io(format!("git apply failed to run: {e}")))?;
let status = if r.exit_code == 0 {
ApplyStatus::Success
} else if !r.applied_paths.is_empty() || !r.conflicted_paths.is_empty() {
ApplyStatus::Partial
} else {
ApplyStatus::Error
};
let applied = matches!(status, ApplyStatus::Success) && !preflight;
let message = if preflight {
match status {
ApplyStatus::Success => {
format!("Preflight passed for task {id} (applies cleanly)")
}
ApplyStatus::Partial => format!(
"Preflight: patch does not fully apply for task {id} (applied={}, skipped={}, conflicts={})",
r.applied_paths.len(),
r.skipped_paths.len(),
r.conflicted_paths.len()
),
ApplyStatus::Error => format!(
"Preflight failed for task {id} (applied={}, skipped={}, conflicts={})",
r.applied_paths.len(),
r.skipped_paths.len(),
r.conflicted_paths.len()
),
}
} else {
match status {
ApplyStatus::Success => format!(
"Applied task {id} locally ({} files)",
r.applied_paths.len()
),
ApplyStatus::Partial => format!(
"Apply partially succeeded for task {id} (applied={}, skipped={}, conflicts={})",
r.applied_paths.len(),
r.skipped_paths.len(),
r.conflicted_paths.len()
),
ApplyStatus::Error => format!(
"Apply failed for task {id} (applied={}, skipped={}, conflicts={})",
r.applied_paths.len(),
r.skipped_paths.len(),
r.conflicted_paths.len()
),
}
};
if matches!(status, ApplyStatus::Partial | ApplyStatus::Error)
|| (preflight && !matches!(status, ApplyStatus::Success))
{
let mut log = String::new();
let summary = summarize_patch_for_logging(&diff);
let mode = if preflight { "preflight" } else { "apply" };
use std::fmt::Write as _;
let _ = writeln!(
&mut log,
"apply_result: mode={} id={} status={:?} applied={} skipped={} conflicts={} cmd={}",
mode,
id,
status,
r.applied_paths.len(),
r.skipped_paths.len(),
r.conflicted_paths.len(),
r.cmd_for_log
);
let _ = writeln!(
&mut log,
"stdout_tail=\n{}\nstderr_tail=\n{}",
tail(&r.stdout, 2000),
tail(&r.stderr, 2000)
);
let _ = writeln!(&mut log, "{summary}");
let _ = writeln!(
&mut log,
"----- PATCH BEGIN -----\n{diff}\n----- PATCH END -----"
);
append_error_log(&log);
}
Ok(ApplyOutcome {
applied,
status,
message,
skipped_paths: r.skipped_paths,
conflict_paths: r.conflicted_paths,
})
}
}
fn details_path(base_url: &str, id: &str) -> Option<String> {
if base_url.contains("/backend-api") {
Some(format!("{base_url}/wham/tasks/{id}"))
} else if base_url.contains("/api/codex") {
Some(format!("{base_url}/tasks/{id}"))
} else {
None
}
}
fn extract_assistant_messages_from_body(body: &str) -> Vec<String> {
let mut msgs = Vec::new();
if let Ok(full) = serde_json::from_str::<serde_json::Value>(body)
&& let Some(arr) = full
.get("current_assistant_turn")
.and_then(|v| v.get("worklog"))
.and_then(|v| v.get("messages"))
.and_then(|v| v.as_array())
{
for m in arr {
let is_assistant = m
.get("author")
.and_then(|a| a.get("role"))
.and_then(|r| r.as_str())
== Some("assistant");
if !is_assistant {
continue;
}
if let Some(parts) = m
.get("content")
.and_then(|c| c.get("parts"))
.and_then(|p| p.as_array())
{
for p in parts {
if let Some(s) = p.as_str() {
if !s.is_empty() {
msgs.push(s.to_string());
}
continue;
}
if let Some(obj) = p.as_object()
&& obj.get("content_type").and_then(|t| t.as_str()) == Some("text")
&& let Some(txt) = obj.get("text").and_then(|t| t.as_str())
{
msgs.push(txt.to_string());
}
}
}
}
}
msgs
}
fn turn_attempt_from_map(turn: &HashMap<String, Value>) -> Option<TurnAttempt> {
let turn_id = turn.get("id").and_then(Value::as_str)?.to_string();
let attempt_placement = turn.get("attempt_placement").and_then(Value::as_i64);
let created_at = parse_timestamp_value(turn.get("created_at"));
let status = attempt_status_from_str(turn.get("turn_status").and_then(Value::as_str));
let diff = extract_diff_from_turn(turn);
let messages = extract_assistant_messages_from_turn(turn);
Some(TurnAttempt {
turn_id,
attempt_placement,
created_at,
status,
diff,
messages,
})
}
fn compare_attempts(a: &TurnAttempt, b: &TurnAttempt) -> Ordering {
match (a.attempt_placement, b.attempt_placement) {
(Some(lhs), Some(rhs)) => lhs.cmp(&rhs),
(Some(_), None) => Ordering::Less,
(None, Some(_)) => Ordering::Greater,
(None, None) => match (a.created_at, b.created_at) {
(Some(lhs), Some(rhs)) => lhs.cmp(&rhs),
(Some(_), None) => Ordering::Less,
(None, Some(_)) => Ordering::Greater,
(None, None) => a.turn_id.cmp(&b.turn_id),
},
}
}
fn extract_diff_from_turn(turn: &HashMap<String, Value>) -> Option<String> {
let items = turn.get("output_items").and_then(Value::as_array)?;
for item in items {
match item.get("type").and_then(Value::as_str) {
Some("output_diff") => {
if let Some(diff) = item.get("diff").and_then(Value::as_str)
&& !diff.is_empty()
{
return Some(diff.to_string());
}
}
Some("pr") => {
if let Some(diff) = item
.get("output_diff")
.and_then(Value::as_object)
.and_then(|od| od.get("diff"))
.and_then(Value::as_str)
&& !diff.is_empty()
{
return Some(diff.to_string());
}
}
_ => {}
}
}
None
}
fn extract_assistant_messages_from_turn(turn: &HashMap<String, Value>) -> Vec<String> {
let mut msgs = Vec::new();
if let Some(items) = turn.get("output_items").and_then(Value::as_array) {
for item in items {
if item.get("type").and_then(Value::as_str) != Some("message") {
continue;
}
if let Some(content) = item.get("content").and_then(Value::as_array) {
for part in content {
if part.get("content_type").and_then(Value::as_str) == Some("text")
&& let Some(txt) = part.get("text").and_then(Value::as_str)
&& !txt.is_empty()
{
msgs.push(txt.to_string());
}
}
}
}
}
msgs
}
fn attempt_status_from_str(raw: Option<&str>) -> AttemptStatus {
match raw.unwrap_or_default() {
"failed" => AttemptStatus::Failed,
"completed" => AttemptStatus::Completed,
"in_progress" => AttemptStatus::InProgress,
"pending" => AttemptStatus::Pending,
_ => AttemptStatus::Pending,
}
}
fn parse_timestamp_value(v: Option<&Value>) -> Option<DateTime<Utc>> {
let ts = v?.as_f64()?;
let secs = ts as i64;
let nanos = ((ts - secs as f64) * 1_000_000_000.0) as u32;
Some(DateTime::<Utc>::from(
std::time::UNIX_EPOCH + std::time::Duration::new(secs.max(0) as u64, nanos),
))
}
fn map_task_list_item_to_summary(src: backend::TaskListItem) -> TaskSummary {
let status_display = src.task_status_display.as_ref();
TaskSummary {
id: TaskId(src.id),
title: src.title,
status: map_status(status_display),
updated_at: parse_updated_at(src.updated_at.as_ref()),
environment_id: None,
environment_label: env_label_from_status_display(status_display),
summary: diff_summary_from_status_display(status_display),
is_review: src
.pull_requests
.as_ref()
.is_some_and(|prs| !prs.is_empty()),
attempt_total: attempt_total_from_status_display(status_display),
}
}
fn map_status(v: Option<&HashMap<String, Value>>) -> TaskStatus {
if let Some(val) = v {
if let Some(turn) = val
.get("latest_turn_status_display")
.and_then(Value::as_object)
&& let Some(s) = turn.get("turn_status").and_then(Value::as_str)
{
return match s {
"failed" => TaskStatus::Error,
"completed" => TaskStatus::Ready,
"in_progress" => TaskStatus::Pending,
"pending" => TaskStatus::Pending,
"cancelled" => TaskStatus::Error,
_ => TaskStatus::Pending,
};
}
if let Some(state) = val.get("state").and_then(Value::as_str) {
return match state {
"pending" => TaskStatus::Pending,
"ready" => TaskStatus::Ready,
"applied" => TaskStatus::Applied,
"error" => TaskStatus::Error,
_ => TaskStatus::Pending,
};
}
}
TaskStatus::Pending
}
fn parse_updated_at(ts: Option<&f64>) -> DateTime<Utc> {
if let Some(v) = ts {
let secs = *v as i64;
let nanos = ((*v - secs as f64) * 1_000_000_000.0) as u32;
return DateTime::<Utc>::from(
std::time::UNIX_EPOCH + std::time::Duration::new(secs.max(0) as u64, nanos),
);
}
Utc::now()
}
fn env_label_from_status_display(v: Option<&HashMap<String, Value>>) -> Option<String> {
let map = v?;
map.get("environment_label")
.and_then(Value::as_str)
.map(str::to_string)
}
fn diff_summary_from_status_display(v: Option<&HashMap<String, Value>>) -> DiffSummary {
let mut out = DiffSummary::default();
let Some(map) = v else { return out };
let latest = map
.get("latest_turn_status_display")
.and_then(Value::as_object);
let Some(latest) = latest else { return out };
if let Some(ds) = latest.get("diff_stats").and_then(Value::as_object) {
if let Some(n) = ds.get("files_modified").and_then(Value::as_i64) {
out.files_changed = n.max(0) as usize;
}
if let Some(n) = ds.get("lines_added").and_then(Value::as_i64) {
out.lines_added = n.max(0) as usize;
}
if let Some(n) = ds.get("lines_removed").and_then(Value::as_i64) {
out.lines_removed = n.max(0) as usize;
}
}
out
}
fn attempt_total_from_status_display(v: Option<&HashMap<String, Value>>) -> Option<usize> {
let map = v?;
let latest = map
.get("latest_turn_status_display")
.and_then(Value::as_object)?;
let siblings = latest.get("sibling_turn_ids").and_then(Value::as_array)?;
Some(siblings.len().saturating_add(1))
}
fn is_unified_diff(diff: &str) -> bool {
let t = diff.trim_start();
if t.starts_with("diff --git ") {
return true;
}
let has_dash_headers = diff.contains("\n--- ") && diff.contains("\n+++ ");
let has_hunk = diff.contains("\n@@ ") || diff.starts_with("@@ ");
has_dash_headers && has_hunk
}
fn tail(s: &str, max: usize) -> String {
if s.len() <= max {
s.to_string()
} else {
s[s.len() - max..].to_string()
}
}
fn summarize_patch_for_logging(patch: &str) -> String {
let trimmed = patch.trim_start();
let kind = if trimmed.starts_with("*** Begin Patch") {
"codex-patch"
} else if trimmed.starts_with("diff --git ") || trimmed.contains("\n*** End Patch\n") {
"git-diff"
} else if trimmed.starts_with("@@ ") || trimmed.contains("\n@@ ") {
"unified-diff"
} else {
"unknown"
};
let lines = patch.lines().count();
let chars = patch.len();
let cwd = std::env::current_dir()
.ok()
.map(|p| p.display().to_string())
.unwrap_or_else(|| "<unknown>".to_string());
let head: String = patch.lines().take(20).collect::<Vec<&str>>().join("\n");
let head_trunc = if head.len() > 800 {
format!("{}…", &head[..800])
} else {
head
};
format!(
"patch_summary: kind={kind} lines={lines} chars={chars} cwd={cwd} ; head=\n{head_trunc}"
)
}
}
fn append_error_log(message: &str) {
let ts = Utc::now().to_rfc3339();
if let Ok(mut f) = std::fs::OpenOptions::new()
.create(true)
.append(true)
.open("error.log")
{
use std::io::Write as _;
let _ = writeln!(f, "[{ts}] {message}");
}
}
code-rs/cloud-tasks-client/src/lib.rs
+2 -29
View File
@@ -1,29 +1,2 @@
mod api;
pub use api::ApplyOutcome;
pub use api::ApplyStatus;
pub use api::AttemptStatus;
pub use api::CloudBackend;
pub use api::CloudTaskError;
pub use api::CreatedTask;
pub use api::DiffSummary;
pub use api::Result;
pub use api::TaskId;
pub use api::TaskStatus;
pub use api::TaskSummary;
pub use api::TaskText;
pub use api::TurnAttempt;
#[cfg(feature = "mock")]
mod mock;
#[cfg(feature = "online")]
mod http;
#[cfg(feature = "mock")]
pub use mock::MockClient;
#[cfg(feature = "online")]
pub use http::HttpClient;
// Reusable apply engine now lives in the shared crate `codex-git-apply`.
//! Thin wrapper around the upstream `codex-cloud-tasks-client` crate.
pub use codex_cloud_tasks_client::*;
code-rs/cloud-tasks-client/src/mock.rs
-180
View File
@@ -1,180 +0,0 @@
use crate::ApplyOutcome;
use crate::AttemptStatus;
use crate::CloudBackend;
use crate::DiffSummary;
use crate::Result;
use crate::TaskId;
use crate::TaskStatus;
use crate::TaskSummary;
use crate::TurnAttempt;
use crate::api::TaskText;
use chrono::Utc;
#[derive(Clone, Default)]
pub struct MockClient;
#[async_trait::async_trait]
impl CloudBackend for MockClient {
async fn list_tasks(&self, _env: Option<&str>) -> Result<Vec<TaskSummary>> {
// Slightly vary content by env to aid tests that rely on the mock
let rows = match _env {
Some("env-A") => vec![("T-2000", "A: First", TaskStatus::Ready)],
Some("env-B") => vec![
("T-3000", "B: One", TaskStatus::Ready),
("T-3001", "B: Two", TaskStatus::Pending),
],
_ => vec![
("T-1000", "Update README formatting", TaskStatus::Ready),
("T-1001", "Fix clippy warnings in core", TaskStatus::Pending),
("T-1002", "Add contributing guide", TaskStatus::Ready),
],
};
let environment_id = _env.map(str::to_string);
let environment_label = match _env {
Some("env-A") => Some("Env A".to_string()),
Some("env-B") => Some("Env B".to_string()),
Some(other) => Some(other.to_string()),
None => Some("Global".to_string()),
};
let mut out = Vec::new();
for (id_str, title, status) in rows {
let id = TaskId(id_str.to_string());
let diff = mock_diff_for(&id);
let (a, d) = count_from_unified(&diff);
out.push(TaskSummary {
id,
title: title.to_string(),
status,
updated_at: Utc::now(),
environment_id: environment_id.clone(),
environment_label: environment_label.clone(),
summary: DiffSummary {
files_changed: 1,
lines_added: a,
lines_removed: d,
},
is_review: false,
attempt_total: Some(if id_str == "T-1000" { 2 } else { 1 }),
});
}
Ok(out)
}
async fn get_task_diff(&self, id: TaskId) -> Result<Option<String>> {
Ok(Some(mock_diff_for(&id)))
}
async fn get_task_messages(&self, _id: TaskId) -> Result<Vec<String>> {
Ok(vec![
"Mock assistant output: this task contains no diff.".to_string(),
])
}
async fn get_task_text(&self, _id: TaskId) -> Result<TaskText> {
Ok(TaskText {
prompt: Some("Why is there no diff?".to_string()),
messages: vec!["Mock assistant output: this task contains no diff.".to_string()],
turn_id: Some("mock-turn".to_string()),
sibling_turn_ids: Vec::new(),
attempt_placement: Some(0),
attempt_status: AttemptStatus::Completed,
})
}
async fn apply_task(&self, id: TaskId, _diff_override: Option<String>) -> Result<ApplyOutcome> {
Ok(ApplyOutcome {
applied: true,
status: crate::ApplyStatus::Success,
message: format!("Applied task {} locally (mock)", id.0),
skipped_paths: Vec::new(),
conflict_paths: Vec::new(),
})
}
async fn apply_task_preflight(
&self,
id: TaskId,
_diff_override: Option<String>,
) -> Result<ApplyOutcome> {
Ok(ApplyOutcome {
applied: false,
status: crate::ApplyStatus::Success,
message: format!("Preflight passed for task {} (mock)", id.0),
skipped_paths: Vec::new(),
conflict_paths: Vec::new(),
})
}
async fn list_sibling_attempts(
&self,
task: TaskId,
_turn_id: String,
) -> Result<Vec<TurnAttempt>> {
if task.0 == "T-1000" {
return Ok(vec![TurnAttempt {
turn_id: "T-1000-attempt-2".to_string(),
attempt_placement: Some(1),
created_at: Some(Utc::now()),
status: AttemptStatus::Completed,
diff: Some(mock_diff_for(&task)),
messages: vec!["Mock alternate attempt".to_string()],
}]);
}
Ok(Vec::new())
}
async fn create_task(
&self,
env_id: &str,
prompt: &str,
git_ref: &str,
qa_mode: bool,
best_of_n: usize,
) -> Result<crate::CreatedTask> {
let _ = (env_id, prompt, git_ref, qa_mode, best_of_n);
let id = format!("task_local_{}", chrono::Utc::now().timestamp_millis());
Ok(crate::CreatedTask { id: TaskId(id) })
}
}
fn mock_diff_for(id: &TaskId) -> String {
match id.0.as_str() {
"T-1000" => {
"diff --git a/README.md b/README.md\nindex 000000..111111 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,2 +1,3 @@\n Intro\n-Hello\n+Hello, world!\n+Task: T-1000\n".to_string()
}
"T-1001" => {
"diff --git a/core/src/lib.rs b/core/src/lib.rs\nindex 000000..111111 100644\n--- a/core/src/lib.rs\n+++ b/core/src/lib.rs\n@@ -1,2 +1,1 @@\n-use foo;\n use bar;\n".to_string()
}
_ => {
"diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md\nindex 000000..111111 100644\n--- /dev/null\n+++ b/CONTRIBUTING.md\n@@ -0,0 +1,3 @@\n+## Contributing\n+Please open PRs.\n+Thanks!\n".to_string()
}
}
}
fn count_from_unified(diff: &str) -> (usize, usize) {
if let Ok(patch) = diffy::Patch::from_str(diff) {
patch
.hunks()
.iter()
.flat_map(diffy::Hunk::lines)
.fold((0, 0), |(a, d), l| match l {
diffy::Line::Insert(_) => (a + 1, d),
diffy::Line::Delete(_) => (a, d + 1),
_ => (a, d),
})
} else {
let mut a = 0;
let mut d = 0;
for l in diff.lines() {
if l.starts_with("+++") || l.starts_with("---") || l.starts_with("@@") {
continue;
}
match l.as_bytes().first() {
Some(b'+') => a += 1,
Some(b'-') => d += 1,
_ => {}
}
}
(a, d)
}
}
code-rs/execpolicy/Cargo.toml
+1 -15
View File
@@ -15,19 +15,5 @@ path = "src/lib.rs"
workspace = true
[dependencies]
allocative = { workspace = true }
anyhow = { workspace = true }
clap = { workspace = true, features = ["derive"] }
derive_more = { workspace = true, features = ["display"] }
env_logger = { workspace = true }
log = { workspace = true }
multimap = { workspace = true }
path-absolutize = { workspace = true }
regex-lite = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_with = { workspace = true, features = ["macros"] }
starlark = { workspace = true }
[dev-dependencies]
tempfile = { workspace = true }
codex-execpolicy = { workspace = true }
code-rs/execpolicy/README.md
-180
View File
@@ -1,180 +0,0 @@
# code_execpolicy
The goal of this library is to classify a proposed [`execv(3)`](https://linux.die.net/man/3/execv) command into one of the following states:
- `safe` The command is safe to run (\*).
- `match` The command matched a rule in the policy, but the caller should decide whether it is safe to run based on the files it will write.
- `forbidden` The command is not allowed to be run.
- `unverified` The safety cannot be determined: make the user decide.
(\*) Whether an `execv(3)` call should be considered "safe" often requires additional context beyond the arguments to `execv()` itself. For example, if you trust an autonomous software agent to write files in your source tree, then deciding whether `/bin/cp foo bar` is "safe" depends on `getcwd(3)` for the calling process as well as the `realpath` of `foo` and `bar` when resolved against `getcwd()`.
To that end, rather than returning a boolean, the validator returns a structured result that the client is expected to use to determine the "safety" of the proposed `execv()` call.
For example, to check the command `ls -l foo`, the checker would be invoked as follows:
```shell
cargo run -- check ls -l foo | jq
```
It will exit with `0` and print the following to stdout:
```json
{
"result": "safe",
"match": {
"program": "ls",
"flags": [
{
"name": "-l"
}
],
"opts": [],
"args": [
{
"index": 1,
"type": "ReadableFile",
"value": "foo"
}
],
"system_path": ["/bin/ls", "/usr/bin/ls"]
}
}
```
Of note:
- `foo` is tagged as a `ReadableFile`, so the caller should resolve `foo` relative to `getcwd()` and `realpath` it (as it may be a symlink) to determine whether `foo` is safe to read.
- While the specified executable is `ls`, `"system_path"` offers `/bin/ls` and `/usr/bin/ls` as viable alternatives to avoid using whatever `ls` happens to appear first on the user's `$PATH`. If either exists on the host, it is recommended to use it as the first argument to `execv(3)` instead of `ls`.
Further, "safety" in this system is not a guarantee that the command will execute successfully. As an example, `cat /Users/mbolin/code/codex/README.md` may be considered "safe" if the system has decided the agent is allowed to read anything under `/Users/mbolin/code/codex`, but it will fail at runtime if `README.md` does not exist. (Though this is "safe" in that the agent did not read any files that it was not authorized to read.)
## Policy
Currently, the default policy is defined in [`default.policy`](./src/default.policy) within the crate.
The system uses [Starlark](https://bazel.build/rules/language) as the file format because, unlike something like JSON or YAML, it supports "macros" without compromising on safety or reproducibility. (Under the hood, we use [`starlark-rust`](https://github.com/facebook/starlark-rust) as the specific Starlark implementation.)
This policy contains "rules" such as:
```python
define_program(
program="cp",
options=[
flag("-r"),
flag("-R"),
flag("--recursive"),
],
args=[ARG_RFILES, ARG_WFILE],
system_path=["/bin/cp", "/usr/bin/cp"],
should_match=[
["foo", "bar"],
],
should_not_match=[
["foo"],
],
)
```
This rule means that:
- `cp` can be used with any of the following flags (where "flag" means "an option that does not take an argument"): `-r`, `-R`, `--recursive`.
- The initial `ARG_RFILES` passed to `args` means that it expects one or more arguments that correspond to "readable files"
- The final `ARG_WFILE` passed to `args` means that it expects exactly one argument that corresponds to a "writeable file."
- As a means of a lightweight way of including a unit test alongside the definition, the `should_match` list is a list of examples of `execv(3)` args that should match the rule and `should_not_match` is a list of examples that should not match. These examples are verified when the `.policy` file is loaded.
Note that the language of the `.policy` file is still evolving, as we have to continue to expand it so it is sufficiently expressive to accept all commands we want to consider "safe" without allowing unsafe commands to pass through.
The integrity of `default.policy` is verified [via unit tests](./tests).
Further, the CLI supports a `--policy` option to specify a custom `.policy` file for ad-hoc testing.
## Output Type: `match`
Going back to the `cp` example, because the rule matches an `ARG_WFILE`, it will return `match` instead of `safe`:
```shell
cargo run -- check cp src1 src2 dest | jq
```
If the caller wants to consider allowing this command, it should parse the JSON to pick out the `WriteableFile` arguments and decide whether they are safe to write:
```json
{
"result": "match",
"match": {
"program": "cp",
"flags": [],
"opts": [],
"args": [
{
"index": 0,
"type": "ReadableFile",
"value": "src1"
},
{
"index": 1,
"type": "ReadableFile",
"value": "src2"
},
{
"index": 2,
"type": "WriteableFile",
"value": "dest"
}
],
"system_path": ["/bin/cp", "/usr/bin/cp"]
}
}
```
Note the exit code is still `0` for a `match` unless the `--require-safe` flag is specified, in which case the exit code is `12`.
## Output Type: `forbidden`
It is also possible to define a rule that, if it matches a command, should flag it as _forbidden_. For example, we do not want agents to be able to run `applied deploy` _ever_, so we define the following rule:
```python
define_program(
program="applied",
args=["deploy"],
forbidden="Infrastructure Risk: command contains 'applied deploy'",
should_match=[
["deploy"],
],
should_not_match=[
["lint"],
],
)
```
Note that for a rule to be forbidden, the `forbidden` keyword arg must be specified as the reason the command is forbidden. This will be included in the output:
```shell
cargo run -- check applied deploy | jq
```
```json
{
"result": "forbidden",
"reason": "Infrastructure Risk: command contains 'applied deploy'",
"cause": {
"Exec": {
"exec": {
"program": "applied",
"flags": [],
"opts": [],
"args": [
{
"index": 0,
"type": {
"Literal": "deploy"
},
"value": "deploy"
}
],
"system_path": []
}
}
}
}
```
code-rs/execpolicy/build.rs
-3
View File
@@ -1,3 +0,0 @@
fn main() {
println!("cargo:rerun-if-changed=src/default.policy");
}
code-rs/execpolicy/src/arg_matcher.rs
-118
View File
@@ -1,118 +0,0 @@
#![allow(clippy::needless_lifetimes)]
use crate::arg_type::ArgType;
use crate::starlark::values::ValueLike;
use allocative::Allocative;
use derive_more::derive::Display;
use starlark::any::ProvidesStaticType;
use starlark::values::AllocValue;
use starlark::values::Heap;
use starlark::values::NoSerialize;
use starlark::values::StarlarkValue;
use starlark::values::UnpackValue;
use starlark::values::Value;
use starlark::values::starlark_value;
use starlark::values::string::StarlarkStr;
/// Patterns that lists of arguments should be compared against.
#[derive(Clone, Debug, Display, Eq, PartialEq, NoSerialize, ProvidesStaticType, Allocative)]
#[display("{}", self)]
pub enum ArgMatcher {
/// Literal string value.
Literal(String),
/// We cannot say what type of value this should match, but it is *not* a file path.
OpaqueNonFile,
/// Required readable file.
ReadableFile,
/// Required writeable file.
WriteableFile,
/// Non-empty list of readable files.
ReadableFiles,
/// Non-empty list of readable files, or empty list, implying readable cwd.
ReadableFilesOrCwd,
/// Positive integer, like one that is required for `head -n`.
PositiveInteger,
/// Bespoke matcher for safe sed commands.
SedCommand,
/// Matches an arbitrary number of arguments without attributing any
/// particular meaning to them. Caller is responsible for interpreting them.
UnverifiedVarargs,
}
impl ArgMatcher {
pub fn cardinality(&self) -> ArgMatcherCardinality {
match self {
ArgMatcher::Literal(_)
| ArgMatcher::OpaqueNonFile
| ArgMatcher::ReadableFile
| ArgMatcher::WriteableFile
| ArgMatcher::PositiveInteger
| ArgMatcher::SedCommand => ArgMatcherCardinality::One,
ArgMatcher::ReadableFiles => ArgMatcherCardinality::AtLeastOne,
ArgMatcher::ReadableFilesOrCwd | ArgMatcher::UnverifiedVarargs => {
ArgMatcherCardinality::ZeroOrMore
}
}
}
pub fn arg_type(&self) -> ArgType {
match self {
ArgMatcher::Literal(value) => ArgType::Literal(value.clone()),
ArgMatcher::OpaqueNonFile => ArgType::OpaqueNonFile,
ArgMatcher::ReadableFile => ArgType::ReadableFile,
ArgMatcher::WriteableFile => ArgType::WriteableFile,
ArgMatcher::ReadableFiles => ArgType::ReadableFile,
ArgMatcher::ReadableFilesOrCwd => ArgType::ReadableFile,
ArgMatcher::PositiveInteger => ArgType::PositiveInteger,
ArgMatcher::SedCommand => ArgType::SedCommand,
ArgMatcher::UnverifiedVarargs => ArgType::Unknown,
}
}
}
pub enum ArgMatcherCardinality {
One,
AtLeastOne,
ZeroOrMore,
}
impl ArgMatcherCardinality {
pub fn is_exact(&self) -> Option<usize> {
match self {
ArgMatcherCardinality::One => Some(1),
ArgMatcherCardinality::AtLeastOne => None,
ArgMatcherCardinality::ZeroOrMore => None,
}
}
}
impl<'v> AllocValue<'v> for ArgMatcher {
fn alloc_value(self, heap: &'v Heap) -> Value<'v> {
heap.alloc_simple(self)
}
}
#[starlark_value(type = "ArgMatcher")]
impl<'v> StarlarkValue<'v> for ArgMatcher {
type Canonical = ArgMatcher;
}
impl<'v> UnpackValue<'v> for ArgMatcher {
type Error = starlark::Error;
fn unpack_value_impl(value: Value<'v>) -> starlark::Result<Option<Self>> {
if let Some(str) = value.downcast_ref::<StarlarkStr>() {
Ok(Some(ArgMatcher::Literal(str.as_str().to_string())))
} else {
Ok(value.downcast_ref::<ArgMatcher>().cloned())
}
}
}
code-rs/execpolicy/src/arg_resolver.rs
-204
View File
@@ -1,204 +0,0 @@
use serde::Serialize;
use crate::arg_matcher::ArgMatcher;
use crate::arg_matcher::ArgMatcherCardinality;
use crate::error::Error;
use crate::error::Result;
use crate::valid_exec::MatchedArg;
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct PositionalArg {
pub index: usize,
pub value: String,
}
pub fn resolve_observed_args_with_patterns(
program: &str,
args: Vec<PositionalArg>,
arg_patterns: &Vec<ArgMatcher>,
) -> Result<Vec<MatchedArg>> {
// Naive matching implementation. Among `arg_patterns`, there is allowed to
// be at most one vararg pattern. Assuming `arg_patterns` is non-empty, we
// end up with either:
//
// - all `arg_patterns` in `prefix_patterns`
// - `arg_patterns` split across `prefix_patterns` (which could be empty),
// one `vararg_pattern`, and `suffix_patterns` (which could also empty).
//
// From there, we start by matching everything in `prefix_patterns`.
// Then we calculate how many positional args should be matched by
// `suffix_patterns` and use that to determine how many args are left to
// be matched by `vararg_pattern` (which could be zero).
//
// After associating positional args with `vararg_pattern`, we match the
// `suffix_patterns` with the remaining args.
let ParitionedArgs {
num_prefix_args,
num_suffix_args,
prefix_patterns,
suffix_patterns,
vararg_pattern,
} = partition_args(program, arg_patterns)?;
let mut matched_args = Vec::<MatchedArg>::new();
let prefix = get_range_checked(&args, 0..num_prefix_args)?;
let mut prefix_arg_index = 0;
for pattern in prefix_patterns {
let n = pattern
.cardinality()
.is_exact()
.ok_or(Error::InternalInvariantViolation {
message: "expected exact cardinality".to_string(),
})?;
for positional_arg in &prefix[prefix_arg_index..prefix_arg_index + n] {
let matched_arg = MatchedArg::new(
positional_arg.index,
pattern.arg_type(),
&positional_arg.value.clone(),
)?;
matched_args.push(matched_arg);
}
prefix_arg_index += n;
}
if num_suffix_args > args.len() {
return Err(Error::NotEnoughArgs {
program: program.to_string(),
args,
arg_patterns: arg_patterns.clone(),
});
}
let initial_suffix_args_index = args.len() - num_suffix_args;
if prefix_arg_index > initial_suffix_args_index {
return Err(Error::PrefixOverlapsSuffix {});
}
if let Some(pattern) = vararg_pattern {
let vararg = get_range_checked(&args, prefix_arg_index..initial_suffix_args_index)?;
match pattern.cardinality() {
ArgMatcherCardinality::One => {
return Err(Error::InternalInvariantViolation {
message: "vararg pattern should not have cardinality of one".to_string(),
});
}
ArgMatcherCardinality::AtLeastOne => {
if vararg.is_empty() {
return Err(Error::VarargMatcherDidNotMatchAnything {
program: program.to_string(),
matcher: pattern,
});
} else {
for positional_arg in vararg {
let matched_arg = MatchedArg::new(
positional_arg.index,
pattern.arg_type(),
&positional_arg.value.clone(),
)?;
matched_args.push(matched_arg);
}
}
}
ArgMatcherCardinality::ZeroOrMore => {
for positional_arg in vararg {
let matched_arg = MatchedArg::new(
positional_arg.index,
pattern.arg_type(),
&positional_arg.value.clone(),
)?;
matched_args.push(matched_arg);
}
}
}
}
let suffix = get_range_checked(&args, initial_suffix_args_index..args.len())?;
let mut suffix_arg_index = 0;
for pattern in suffix_patterns {
let n = pattern
.cardinality()
.is_exact()
.ok_or(Error::InternalInvariantViolation {
message: "expected exact cardinality".to_string(),
})?;
for positional_arg in &suffix[suffix_arg_index..suffix_arg_index + n] {
let matched_arg = MatchedArg::new(
positional_arg.index,
pattern.arg_type(),
&positional_arg.value.clone(),
)?;
matched_args.push(matched_arg);
}
suffix_arg_index += n;
}
if matched_args.len() < args.len() {
let extra_args = get_range_checked(&args, matched_args.len()..args.len())?;
Err(Error::UnexpectedArguments {
program: program.to_string(),
args: extra_args.to_vec(),
})
} else {
Ok(matched_args)
}
}
#[derive(Default)]
struct ParitionedArgs {
num_prefix_args: usize,
num_suffix_args: usize,
prefix_patterns: Vec<ArgMatcher>,
suffix_patterns: Vec<ArgMatcher>,
vararg_pattern: Option<ArgMatcher>,
}
fn partition_args(program: &str, arg_patterns: &Vec<ArgMatcher>) -> Result<ParitionedArgs> {
let mut in_prefix = true;
let mut partitioned_args = ParitionedArgs::default();
for pattern in arg_patterns {
match pattern.cardinality().is_exact() {
Some(n) => {
if in_prefix {
partitioned_args.prefix_patterns.push(pattern.clone());
partitioned_args.num_prefix_args += n;
} else {
partitioned_args.suffix_patterns.push(pattern.clone());
partitioned_args.num_suffix_args += n;
}
}
None => match partitioned_args.vararg_pattern {
None => {
partitioned_args.vararg_pattern = Some(pattern.clone());
in_prefix = false;
}
Some(existing_pattern) => {
return Err(Error::MultipleVarargPatterns {
program: program.to_string(),
first: existing_pattern,
second: pattern.clone(),
});
}
},
}
}
Ok(partitioned_args)
}
fn get_range_checked<T>(vec: &[T], range: std::ops::Range<usize>) -> Result<&[T]> {
if range.start > range.end {
Err(Error::RangeStartExceedsEnd {
start: range.start,
end: range.end,
})
} else if range.end > vec.len() {
Err(Error::RangeEndOutOfBounds {
end: range.end,
len: vec.len(),
})
} else {
Ok(&vec[range])
}
}
code-rs/execpolicy/src/arg_type.rs
-87
View File
@@ -1,87 +0,0 @@
#![allow(clippy::needless_lifetimes)]
use crate::error::Error;
use crate::error::Result;
use crate::sed_command::parse_sed_command;
use allocative::Allocative;
use derive_more::derive::Display;
use serde::Serialize;
use starlark::any::ProvidesStaticType;
use starlark::values::StarlarkValue;
use starlark::values::starlark_value;
#[derive(Debug, Clone, Display, Eq, PartialEq, ProvidesStaticType, Allocative, Serialize)]
#[display("{}", self)]
pub enum ArgType {
Literal(String),
/// We cannot say what this argument represents, but it is *not* a file path.
OpaqueNonFile,
/// A file (or directory) that can be expected to be read as part of this command.
ReadableFile,
/// A file (or directory) that can be expected to be written as part of this command.
WriteableFile,
/// Positive integer, like one that is required for `head -n`.
PositiveInteger,
/// Bespoke arg type for a safe sed command.
SedCommand,
/// Type is unknown: it may or may not be a file.
Unknown,
}
impl ArgType {
pub fn validate(&self, value: &str) -> Result<()> {
match self {
ArgType::Literal(literal_value) => {
if value != *literal_value {
Err(Error::LiteralValueDidNotMatch {
expected: literal_value.clone(),
actual: value.to_string(),
})
} else {
Ok(())
}
}
ArgType::ReadableFile => {
if value.is_empty() {
Err(Error::EmptyFileName {})
} else {
Ok(())
}
}
ArgType::WriteableFile => {
if value.is_empty() {
Err(Error::EmptyFileName {})
} else {
Ok(())
}
}
ArgType::OpaqueNonFile | ArgType::Unknown => Ok(()),
ArgType::PositiveInteger => match value.parse::<u64>() {
Ok(0) => Err(Error::InvalidPositiveInteger {
value: value.to_string(),
}),
Ok(_) => Ok(()),
Err(_) => Err(Error::InvalidPositiveInteger {
value: value.to_string(),
}),
},
ArgType::SedCommand => parse_sed_command(value),
}
}
pub fn might_write_file(&self) -> bool {
match self {
ArgType::WriteableFile | ArgType::Unknown => true,
ArgType::Literal(_)
| ArgType::OpaqueNonFile
| ArgType::PositiveInteger
| ArgType::ReadableFile
| ArgType::SedCommand => false,
}
}
}
#[starlark_value(type = "ArgType")]
impl<'v> StarlarkValue<'v> for ArgType {
type Canonical = ArgType;
}
code-rs/execpolicy/src/default.policy
-202
View File
@@ -1,202 +0,0 @@
"""
define_program() supports the following arguments:
- program: the name of the program
- system_path: list of absolute paths on the system where program can likely be found
- option_bundling (PLANNED): whether to allow bundling of options (e.g. `-al` for `-a -l`)
- combine_format (PLANNED): whether to allow `--option=value` (as opposed to `--option value`)
- options: the command-line flags/options: use flag() and opt() to define these
- args: the rules for what arguments are allowed that are not "options"
- should_match: list of command-line invocations that should be matched by the rule
- should_not_match: list of command-line invocations that should not be matched by the rule
"""
define_program(
program="ls",
system_path=["/bin/ls", "/usr/bin/ls"],
options=[
flag("-1"),
flag("-a"),
flag("-l"),
],
args=[ARG_RFILES_OR_CWD],
)
define_program(
program="cat",
options=[
flag("-b"),
flag("-n"),
flag("-t"),
],
system_path=["/bin/cat", "/usr/bin/cat"],
args=[ARG_RFILES],
should_match=[
["file.txt"],
["-n", "file.txt"],
["-b", "file.txt"],
],
should_not_match=[
# While cat without args is valid, it will read from stdin, which
# does not seem appropriate for our current use case.
[],
# Let's not auto-approve advisory locking.
["-l", "file.txt"],
]
)
define_program(
program="cp",
options=[
flag("-r"),
flag("-R"),
flag("--recursive"),
],
args=[ARG_RFILES, ARG_WFILE],
system_path=["/bin/cp", "/usr/bin/cp"],
should_match=[
["foo", "bar"],
],
should_not_match=[
["foo"],
],
)
define_program(
program="head",
system_path=["/bin/head", "/usr/bin/head"],
options=[
opt("-c", ARG_POS_INT),
opt("-n", ARG_POS_INT),
],
args=[ARG_RFILES],
)
printenv_system_path = ["/usr/bin/printenv"]
# Print all environment variables.
define_program(
program="printenv",
args=[],
system_path=printenv_system_path,
# This variant of `printenv` only allows zero args.
should_match=[[]],
should_not_match=[["PATH"]],
)
# Print a specific environment variable.
define_program(
program="printenv",
args=[ARG_OPAQUE_VALUE],
system_path=printenv_system_path,
# This variant of `printenv` only allows exactly one arg.
should_match=[["PATH"]],
should_not_match=[[], ["PATH", "HOME"]],
)
# Note that `pwd` is generally implemented as a shell built-in. It does not
# accept any arguments.
define_program(
program="pwd",
options=[
flag("-L"),
flag("-P"),
],
args=[],
)
define_program(
program="rg",
options=[
opt("-A", ARG_POS_INT),
opt("-B", ARG_POS_INT),
opt("-C", ARG_POS_INT),
opt("-d", ARG_POS_INT),
opt("--max-depth", ARG_POS_INT),
opt("-g", ARG_OPAQUE_VALUE),
opt("--glob", ARG_OPAQUE_VALUE),
opt("-m", ARG_POS_INT),
opt("--max-count", ARG_POS_INT),
flag("-n"),
flag("-i"),
flag("-l"),
flag("--files"),
flag("--files-with-matches"),
flag("--files-without-match"),
],
args=[ARG_OPAQUE_VALUE, ARG_RFILES_OR_CWD],
should_match=[
["-n", "init"],
["-n", "init", "."],
["-i", "-n", "init", "src"],
["--files", "--max-depth", "2", "."],
],
should_not_match=[
["-m", "-n", "init"],
["--glob", "src"],
],
# TODO(mbolin): Perhaps we need a way to indicate that we expect `rg` to be
# bundled with the host environment and we should be using that version.
system_path=[],
)
# Unfortunately, `sed` is difficult to secure because GNU sed supports an `e`
# flag where `s/pattern/replacement/e` would run `replacement` as a shell
# command every time `pattern` is matched. For example, try the following on
# Ubuntu (which uses GNU sed, unlike macOS):
#
# ```shell
# $ yes | head -n 4 > /tmp/yes.txt
# $ sed 's/y/echo hi/e' /tmp/yes.txt
# hi
# hi
# hi
# hi
# ```
#
# As you can see, `echo hi` got executed four times. In order to support some
# basic sed functionality, we implement a bespoke `ARG_SED_COMMAND` that matches
# only "known safe" sed commands.
common_sed_flags = [
# We deliberately do not support -i or -f.
flag("-n"),
flag("-u"),
]
sed_system_path = ["/usr/bin/sed"]
# When -e is not specified, the first argument must be a valid sed command.
define_program(
program="sed",
options=common_sed_flags,
args=[ARG_SED_COMMAND, ARG_RFILES],
system_path=sed_system_path,
)
# When -e is required, all arguments are assumed to be readable files.
define_program(
program="sed",
options=common_sed_flags + [
opt("-e", ARG_SED_COMMAND, required=True),
],
args=[ARG_RFILES],
system_path=sed_system_path,
)
define_program(
program="which",
options=[
flag("-a"),
flag("-s"),
],
# Surprisingly, `which` takes more than one argument.
args=[ARG_RFILES],
should_match=[
["python3"],
["-a", "python3"],
["-a", "python3", "cargo"],
],
should_not_match=[
[],
],
system_path=["/bin/which", "/usr/bin/which"],
)
code-rs/execpolicy/src/error.rs
-96
View File
@@ -1,96 +0,0 @@
use std::path::PathBuf;
use serde::Serialize;
use crate::arg_matcher::ArgMatcher;
use crate::arg_resolver::PositionalArg;
use serde_with::DisplayFromStr;
use serde_with::serde_as;
pub type Result<T> = std::result::Result<T, Error>;
#[serde_as]
#[derive(Debug, Eq, PartialEq, Serialize)]
#[serde(tag = "type")]
pub enum Error {
NoSpecForProgram {
program: String,
},
OptionMissingValue {
program: String,
option: String,
},
OptionFollowedByOptionInsteadOfValue {
program: String,
option: String,
value: String,
},
UnknownOption {
program: String,
option: String,
},
UnexpectedArguments {
program: String,
args: Vec<PositionalArg>,
},
DoubleDashNotSupportedYet {
program: String,
},
MultipleVarargPatterns {
program: String,
first: ArgMatcher,
second: ArgMatcher,
},
RangeStartExceedsEnd {
start: usize,
end: usize,
},
RangeEndOutOfBounds {
end: usize,
len: usize,
},
PrefixOverlapsSuffix {},
NotEnoughArgs {
program: String,
args: Vec<PositionalArg>,
arg_patterns: Vec<ArgMatcher>,
},
InternalInvariantViolation {
message: String,
},
VarargMatcherDidNotMatchAnything {
program: String,
matcher: ArgMatcher,
},
EmptyFileName {},
LiteralValueDidNotMatch {
expected: String,
actual: String,
},
InvalidPositiveInteger {
value: String,
},
MissingRequiredOptions {
program: String,
options: Vec<String>,
},
SedCommandNotProvablySafe {
command: String,
},
ReadablePathNotInReadableFolders {
file: PathBuf,
folders: Vec<PathBuf>,
},
WriteablePathNotInWriteableFolders {
file: PathBuf,
folders: Vec<PathBuf>,
},
CannotCheckRelativePath {
file: PathBuf,
},
CannotCanonicalizePath {
file: String,
#[serde_as(as = "DisplayFromStr")]
error: std::io::ErrorKind,
},
}
code-rs/execpolicy/src/exec_call.rs
-28
View File
@@ -1,28 +0,0 @@
use std::fmt::Display;
use serde::Serialize;
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct ExecCall {
pub program: String,
pub args: Vec<String>,
}
impl ExecCall {
pub fn new(program: &str, args: &[&str]) -> Self {
Self {
program: program.to_string(),
args: args.iter().map(|&s| s.into()).collect(),
}
}
}
impl Display for ExecCall {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.program)?;
for arg in &self.args {
write!(f, " {arg}")?;
}
Ok(())
}
}
code-rs/execpolicy/src/execv_checker.rs
-294
View File
@@ -1,294 +0,0 @@
use std::borrow::Cow;
use std::ffi::OsString;
use std::path::Path;
use std::path::PathBuf;
use crate::ArgType;
use crate::Error::CannotCanonicalizePath;
use crate::Error::CannotCheckRelativePath;
use crate::Error::ReadablePathNotInReadableFolders;
use crate::Error::WriteablePathNotInWriteableFolders;
use crate::ExecCall;
use crate::MatchedExec;
use crate::Policy;
use crate::Result;
use crate::ValidExec;
use path_absolutize::*;
macro_rules! check_file_in_folders {
($file:expr, $folders:expr, $error:ident) => {
if !$folders.iter().any(|folder| $file.starts_with(folder)) {
return Err($error {
file: $file.clone(),
folders: $folders.to_vec(),
});
}
};
}
pub struct ExecvChecker {
execv_policy: Policy,
}
impl ExecvChecker {
pub fn new(execv_policy: Policy) -> Self {
Self { execv_policy }
}
pub fn r#match(&self, exec_call: &ExecCall) -> Result<MatchedExec> {
self.execv_policy.check(exec_call)
}
/// The caller is responsible for ensuring readable_folders and
/// writeable_folders are in canonical form.
pub fn check(
&self,
valid_exec: ValidExec,
cwd: &Option<OsString>,
readable_folders: &[PathBuf],
writeable_folders: &[PathBuf],
) -> Result<String> {
for (arg_type, value) in valid_exec
.args
.into_iter()
.map(|arg| (arg.r#type, arg.value))
.chain(
valid_exec
.opts
.into_iter()
.map(|opt| (opt.r#type, opt.value)),
)
{
match arg_type {
ArgType::ReadableFile => {
let readable_file = ensure_absolute_path(&value, cwd)?;
check_file_in_folders!(
readable_file,
readable_folders,
ReadablePathNotInReadableFolders
);
}
ArgType::WriteableFile => {
let writeable_file = ensure_absolute_path(&value, cwd)?;
check_file_in_folders!(
writeable_file,
writeable_folders,
WriteablePathNotInWriteableFolders
);
}
ArgType::OpaqueNonFile
| ArgType::Unknown
| ArgType::PositiveInteger
| ArgType::SedCommand
| ArgType::Literal(_) => {
continue;
}
}
}
let mut program = valid_exec.program.to_string();
for system_path in valid_exec.system_path {
if is_executable_file(&system_path) {
program = system_path;
break;
}
}
Ok(program)
}
}
fn ensure_absolute_path(path: &str, cwd: &Option<OsString>) -> Result<PathBuf> {
let file = PathBuf::from(path);
let result = if file.is_relative() {
match cwd {
Some(cwd) => file.absolutize_from(cwd),
None => return Err(CannotCheckRelativePath { file }),
}
} else {
file.absolutize()
};
result
.map(Cow::into_owned)
.map_err(|error| CannotCanonicalizePath {
file: path.to_string(),
error: error.kind(),
})
}
fn is_executable_file(path: &str) -> bool {
let file_path = Path::new(path);
if let Ok(metadata) = std::fs::metadata(file_path) {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let permissions = metadata.permissions();
// Check if the file is executable (by checking the executable bit for the owner)
return metadata.is_file() && (permissions.mode() & 0o111 != 0);
}
#[cfg(windows)]
{
// TODO(mbolin): Check against PATHEXT environment variable.
return metadata.is_file();
}
}
false
}
#[cfg(test)]
mod tests {
use tempfile::TempDir;
use super::*;
use crate::MatchedArg;
use crate::PolicyParser;
use anyhow::Result;
use anyhow::anyhow;
fn setup(fake_cp: &Path) -> ExecvChecker {
let source = format!(
r#"
define_program(
program="cp",
args=[ARG_RFILE, ARG_WFILE],
system_path=[{fake_cp:?}]
)
"#
);
let parser = PolicyParser::new("#test", &source);
let policy = parser.parse().unwrap();
ExecvChecker::new(policy)
}
#[test]
fn test_check_valid_input_files() -> Result<()> {
let temp_dir = TempDir::new()?;
// Create an executable file that can be used with the system_path arg.
let fake_cp = temp_dir.path().join("cp");
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let fake_cp_file = std::fs::File::create(&fake_cp)?;
let mut permissions = fake_cp_file.metadata()?.permissions();
permissions.set_mode(0o755);
std::fs::set_permissions(&fake_cp, permissions)?;
}
#[cfg(windows)]
{
std::fs::File::create(&fake_cp)?;
}
// Create root_path and reference to files under the root.
let root_path = temp_dir.path().to_path_buf();
let source_path = root_path.join("source");
let dest_path = root_path.join("dest");
let cp = fake_cp.to_str().unwrap().to_string();
let root = root_path.to_str().unwrap().to_string();
let source = source_path.to_str().unwrap().to_string();
let dest = dest_path.to_str().unwrap().to_string();
let cwd = Some(root_path.clone().into());
let checker = setup(&fake_cp);
let exec_call = ExecCall {
program: "cp".into(),
args: vec![source, dest.clone()],
};
let valid_exec = match checker.r#match(&exec_call).map_err(|e| anyhow!("{e:?}"))? {
MatchedExec::Match { exec } => exec,
unexpected => panic!("Expected a safe exec but got {unexpected:?}"),
};
// No readable or writeable folders specified.
assert_eq!(
checker.check(valid_exec.clone(), &cwd, &[], &[]),
Err(ReadablePathNotInReadableFolders {
file: source_path,
folders: vec![]
}),
);
// Only readable folders specified.
assert_eq!(
checker.check(
valid_exec.clone(),
&cwd,
std::slice::from_ref(&root_path),
&[]
),
Err(WriteablePathNotInWriteableFolders {
file: dest_path.clone(),
folders: vec![]
}),
);
// Both readable and writeable folders specified.
assert_eq!(
checker.check(
valid_exec,
&cwd,
std::slice::from_ref(&root_path),
std::slice::from_ref(&root_path)
),
Ok(cp.clone()),
);
// Args are the readable and writeable folders, not files within the
// folders.
let exec_call_folders_as_args = ExecCall {
program: "cp".into(),
args: vec![root.clone(), root],
};
let valid_exec_call_folders_as_args = match checker
.r#match(&exec_call_folders_as_args)
.map_err(|e| anyhow!("{e:?}"))?
{
MatchedExec::Match { exec } => exec,
_ => panic!("Expected a safe exec"),
};
assert_eq!(
checker.check(
valid_exec_call_folders_as_args,
&cwd,
std::slice::from_ref(&root_path),
std::slice::from_ref(&root_path)
),
Ok(cp),
);
// Specify a parent of a readable folder as input.
let exec_with_parent_of_readable_folder = ValidExec {
program: "cp".into(),
args: vec![
MatchedArg::new(
0,
ArgType::ReadableFile,
root_path.parent().unwrap().to_str().unwrap(),
)
.map_err(|e| anyhow!("{e:?}"))?,
MatchedArg::new(1, ArgType::WriteableFile, &dest).map_err(|e| anyhow!("{e:?}"))?,
],
..Default::default()
};
assert_eq!(
checker.check(
exec_with_parent_of_readable_folder,
&cwd,
std::slice::from_ref(&root_path),
std::slice::from_ref(&dest_path)
),
Err(ReadablePathNotInReadableFolders {
file: root_path.parent().unwrap().to_path_buf(),
folders: vec![root_path.clone()]
}),
);
Ok(())
}
}
code-rs/execpolicy/src/lib.rs
+2 -45
View File
@@ -1,45 +1,2 @@
#![allow(clippy::type_complexity)]
#![allow(clippy::too_many_arguments)]
#[macro_use]
extern crate starlark;
mod arg_matcher;
mod arg_resolver;
mod arg_type;
mod error;
mod exec_call;
mod execv_checker;
mod opt;
mod policy;
mod policy_parser;
mod program;
mod sed_command;
mod valid_exec;
pub use arg_matcher::ArgMatcher;
pub use arg_resolver::PositionalArg;
pub use arg_type::ArgType;
pub use error::Error;
pub use error::Result;
pub use exec_call::ExecCall;
pub use execv_checker::ExecvChecker;
pub use opt::Opt;
pub use policy::Policy;
pub use policy_parser::PolicyParser;
pub use program::Forbidden;
pub use program::MatchedExec;
pub use program::NegativeExamplePassedCheck;
pub use program::PositiveExampleFailedCheck;
pub use program::ProgramSpec;
pub use sed_command::parse_sed_command;
pub use valid_exec::MatchedArg;
pub use valid_exec::MatchedFlag;
pub use valid_exec::MatchedOpt;
pub use valid_exec::ValidExec;
const DEFAULT_POLICY: &str = include_str!("default.policy");
pub fn get_default_policy() -> starlark::Result<Policy> {
let parser = PolicyParser::new("#default", DEFAULT_POLICY);
parser.parse()
}
//! Thin wrapper around the upstream `codex-execpolicy` crate.
pub use codex_execpolicy::*;
code-rs/execpolicy/src/main.rs
+2 -166
View File
@@ -1,167 +1,3 @@
use anyhow::Result;
use clap::Parser;
use clap::Subcommand;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedExec;
use code_execpolicy::Policy;
use code_execpolicy::PolicyParser;
use code_execpolicy::ValidExec;
use code_execpolicy::get_default_policy;
use serde::Deserialize;
use serde::Serialize;
use serde::de;
use starlark::Error as StarlarkError;
use std::path::PathBuf;
use std::str::FromStr;
const MATCHED_BUT_WRITES_FILES_EXIT_CODE: i32 = 12;
const MIGHT_BE_SAFE_EXIT_CODE: i32 = 13;
const FORBIDDEN_EXIT_CODE: i32 = 14;
#[derive(Parser, Deserialize, Debug)]
#[command(version, about, long_about = None)]
pub struct Args {
/// If the command fails the policy, exit with 13, but print parseable JSON
/// to stdout.
#[clap(long)]
pub require_safe: bool,
/// Path to the policy file.
#[clap(long, short = 'p')]
pub policy: Option<PathBuf>,
#[command(subcommand)]
pub command: Command,
}
#[derive(Clone, Debug, Deserialize, Subcommand)]
pub enum Command {
/// Checks the command as if the arguments were the inputs to execv(3).
Check {
#[arg(trailing_var_arg = true)]
command: Vec<String>,
},
/// Checks the command encoded as a JSON object.
#[clap(name = "check-json")]
CheckJson {
/// JSON object with "program" (str) and "args" (list[str]) fields.
#[serde(deserialize_with = "deserialize_from_json")]
exec: ExecArg,
},
}
#[derive(Clone, Debug, Deserialize)]
pub struct ExecArg {
pub program: String,
#[serde(default)]
pub args: Vec<String>,
}
fn main() -> Result<()> {
env_logger::init();
let args = Args::parse();
let policy = match args.policy {
Some(policy) => {
let policy_source = policy.to_string_lossy().to_string();
let unparsed_policy = std::fs::read_to_string(policy)?;
let parser = PolicyParser::new(&policy_source, &unparsed_policy);
parser.parse()
}
None => get_default_policy(),
};
let policy = policy.map_err(StarlarkError::into_anyhow)?;
let exec = match args.command {
Command::Check { command } => match command.split_first() {
Some((first, rest)) => ExecArg {
program: first.to_string(),
args: rest.to_vec(),
},
None => {
eprintln!("no command provided");
std::process::exit(1);
}
},
Command::CheckJson { exec } => exec,
};
let (output, exit_code) = check_command(&policy, exec, args.require_safe);
let json = serde_json::to_string(&output)?;
println!("{json}");
std::process::exit(exit_code);
}
fn check_command(
policy: &Policy,
ExecArg { program, args }: ExecArg,
check: bool,
) -> (Output, i32) {
let exec_call = ExecCall { program, args };
match policy.check(&exec_call) {
Ok(MatchedExec::Match { exec }) => {
if exec.might_write_files() {
let exit_code = if check {
MATCHED_BUT_WRITES_FILES_EXIT_CODE
} else {
0
};
(Output::Match { r#match: exec }, exit_code)
} else {
(Output::Safe { r#match: exec }, 0)
}
}
Ok(MatchedExec::Forbidden { reason, cause }) => {
let exit_code = if check { FORBIDDEN_EXIT_CODE } else { 0 };
(Output::Forbidden { reason, cause }, exit_code)
}
Err(err) => {
let exit_code = if check { MIGHT_BE_SAFE_EXIT_CODE } else { 0 };
(Output::Unverified { error: err }, exit_code)
}
}
}
#[derive(Debug, Serialize)]
#[serde(tag = "result")]
pub enum Output {
/// The command is verified as safe.
#[serde(rename = "safe")]
Safe { r#match: ValidExec },
/// The command has matched a rule in the policy, but the caller should
/// decide whether it is "safe" given the files it wants to write.
#[serde(rename = "match")]
Match { r#match: ValidExec },
/// The user is forbidden from running the command.
#[serde(rename = "forbidden")]
Forbidden {
reason: String,
cause: code_execpolicy::Forbidden,
},
/// The safety of the command could not be verified.
#[serde(rename = "unverified")]
Unverified { error: code_execpolicy::Error },
}
fn deserialize_from_json<'de, D>(deserializer: D) -> Result<ExecArg, D::Error>
where
D: de::Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
let decoded = serde_json::from_str(&s)
.map_err(|e| serde::de::Error::custom(format!("JSON parse error: {e}")))?;
Ok(decoded)
}
impl FromStr for ExecArg {
type Err = anyhow::Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
serde_json::from_str(s).map_err(Into::into)
}
fn main() -> anyhow::Result<()> {
codex_execpolicy::main()
}
code-rs/execpolicy/src/opt.rs
-77
View File
@@ -1,77 +0,0 @@
#![allow(clippy::needless_lifetimes)]
use crate::ArgType;
use crate::starlark::values::ValueLike;
use allocative::Allocative;
use derive_more::derive::Display;
use starlark::any::ProvidesStaticType;
use starlark::values::AllocValue;
use starlark::values::Heap;
use starlark::values::NoSerialize;
use starlark::values::StarlarkValue;
use starlark::values::UnpackValue;
use starlark::values::Value;
use starlark::values::starlark_value;
/// Command line option that takes a value.
#[derive(Clone, Debug, Display, PartialEq, Eq, ProvidesStaticType, NoSerialize, Allocative)]
#[display("opt({})", opt)]
pub struct Opt {
/// The option as typed on the command line, e.g., `-h` or `--help`. If
/// it can be used in the `--name=value` format, then this should be
/// `--name` (though this is subject to change).
pub opt: String,
pub meta: OptMeta,
pub required: bool,
}
/// When defining an Opt, use as specific an OptMeta as possible.
#[derive(Clone, Debug, Display, PartialEq, Eq, ProvidesStaticType, NoSerialize, Allocative)]
#[display("{}", self)]
pub enum OptMeta {
/// Option does not take a value.
Flag,
/// Option takes a single value matching the specified type.
Value(ArgType),
}
impl Opt {
pub fn new(opt: String, meta: OptMeta, required: bool) -> Self {
Self {
opt,
meta,
required,
}
}
pub fn name(&self) -> &str {
&self.opt
}
}
#[starlark_value(type = "Opt")]
impl<'v> StarlarkValue<'v> for Opt {
type Canonical = Opt;
}
impl<'v> UnpackValue<'v> for Opt {
type Error = starlark::Error;
fn unpack_value_impl(value: Value<'v>) -> starlark::Result<Option<Self>> {
// TODO(mbolin): It fels like this should be doable without cloning?
// Cannot simply consume the value?
Ok(value.downcast_ref::<Opt>().cloned())
}
}
impl<'v> AllocValue<'v> for Opt {
fn alloc_value(self, heap: &'v Heap) -> Value<'v> {
heap.alloc_simple(self)
}
}
#[starlark_value(type = "OptMeta")]
impl<'v> StarlarkValue<'v> for OptMeta {
type Canonical = OptMeta;
}
code-rs/execpolicy/src/policy.rs
-103
View File
@@ -1,103 +0,0 @@
use multimap::MultiMap;
use regex_lite::Error as RegexError;
use regex_lite::Regex;
use crate::ExecCall;
use crate::Forbidden;
use crate::MatchedExec;
use crate::NegativeExamplePassedCheck;
use crate::ProgramSpec;
use crate::error::Error;
use crate::error::Result;
use crate::policy_parser::ForbiddenProgramRegex;
use crate::program::PositiveExampleFailedCheck;
pub struct Policy {
programs: MultiMap<String, ProgramSpec>,
forbidden_program_regexes: Vec<ForbiddenProgramRegex>,
forbidden_substrings_pattern: Option<Regex>,
}
impl Policy {
pub fn new(
programs: MultiMap<String, ProgramSpec>,
forbidden_program_regexes: Vec<ForbiddenProgramRegex>,
forbidden_substrings: Vec<String>,
) -> std::result::Result<Self, RegexError> {
let forbidden_substrings_pattern = if forbidden_substrings.is_empty() {
None
} else {
let escaped_substrings = forbidden_substrings
.iter()
.map(|s| regex_lite::escape(s))
.collect::<Vec<_>>()
.join("|");
Some(Regex::new(&format!("({escaped_substrings})"))?)
};
Ok(Self {
programs,
forbidden_program_regexes,
forbidden_substrings_pattern,
})
}
pub fn check(&self, exec_call: &ExecCall) -> Result<MatchedExec> {
let ExecCall { program, args } = &exec_call;
for ForbiddenProgramRegex { regex, reason } in &self.forbidden_program_regexes {
if regex.is_match(program) {
return Ok(MatchedExec::Forbidden {
cause: Forbidden::Program {
program: program.clone(),
exec_call: exec_call.clone(),
},
reason: reason.clone(),
});
}
}
for arg in args {
if let Some(regex) = &self.forbidden_substrings_pattern
&& regex.is_match(arg)
{
return Ok(MatchedExec::Forbidden {
cause: Forbidden::Arg {
arg: arg.clone(),
exec_call: exec_call.clone(),
},
reason: format!("arg `{arg}` contains forbidden substring"),
});
}
}
let mut last_err = Err(Error::NoSpecForProgram {
program: program.clone(),
});
if let Some(spec_list) = self.programs.get_vec(program) {
for spec in spec_list {
match spec.check(exec_call) {
Ok(matched_exec) => return Ok(matched_exec),
Err(err) => {
last_err = Err(err);
}
}
}
}
last_err
}
pub fn check_each_good_list_individually(&self) -> Vec<PositiveExampleFailedCheck> {
let mut violations = Vec::new();
for (_program, spec) in self.programs.flat_iter() {
violations.extend(spec.verify_should_match_list());
}
violations
}
pub fn check_each_bad_list_individually(&self) -> Vec<NegativeExamplePassedCheck> {
let mut violations = Vec::new();
for (_program, spec) in self.programs.flat_iter() {
violations.extend(spec.verify_should_not_match_list());
}
violations
}
}
code-rs/execpolicy/src/policy_parser.rs
-226
View File
@@ -1,226 +0,0 @@
#![allow(clippy::needless_lifetimes)]
use crate::Opt;
use crate::Policy;
use crate::ProgramSpec;
use crate::arg_matcher::ArgMatcher;
use crate::opt::OptMeta;
use log::info;
use multimap::MultiMap;
use regex_lite::Regex;
use starlark::any::ProvidesStaticType;
use starlark::environment::GlobalsBuilder;
use starlark::environment::LibraryExtension;
use starlark::environment::Module;
use starlark::eval::Evaluator;
use starlark::syntax::AstModule;
use starlark::syntax::Dialect;
use starlark::values::Heap;
use starlark::values::list::UnpackList;
use starlark::values::none::NoneType;
use std::cell::RefCell;
use std::collections::HashMap;
pub struct PolicyParser {
policy_source: String,
unparsed_policy: String,
}
impl PolicyParser {
pub fn new(policy_source: &str, unparsed_policy: &str) -> Self {
Self {
policy_source: policy_source.to_string(),
unparsed_policy: unparsed_policy.to_string(),
}
}
pub fn parse(&self) -> starlark::Result<Policy> {
let mut dialect = Dialect::Extended.clone();
dialect.enable_f_strings = true;
let ast = AstModule::parse(&self.policy_source, self.unparsed_policy.clone(), &dialect)?;
let globals = GlobalsBuilder::extended_by(&[LibraryExtension::Typing])
.with(policy_builtins)
.build();
let module = Module::new();
let heap = Heap::new();
module.set("ARG_OPAQUE_VALUE", heap.alloc(ArgMatcher::OpaqueNonFile));
module.set("ARG_RFILE", heap.alloc(ArgMatcher::ReadableFile));
module.set("ARG_WFILE", heap.alloc(ArgMatcher::WriteableFile));
module.set("ARG_RFILES", heap.alloc(ArgMatcher::ReadableFiles));
module.set(
"ARG_RFILES_OR_CWD",
heap.alloc(ArgMatcher::ReadableFilesOrCwd),
);
module.set("ARG_POS_INT", heap.alloc(ArgMatcher::PositiveInteger));
module.set("ARG_SED_COMMAND", heap.alloc(ArgMatcher::SedCommand));
module.set(
"ARG_UNVERIFIED_VARARGS",
heap.alloc(ArgMatcher::UnverifiedVarargs),
);
let policy_builder = PolicyBuilder::new();
{
let mut eval = Evaluator::new(&module);
eval.extra = Some(&policy_builder);
eval.eval_module(ast, &globals)?;
}
let policy = policy_builder.build();
policy.map_err(|e| starlark::Error::new_kind(starlark::ErrorKind::Other(e.into())))
}
}
#[derive(Debug)]
pub struct ForbiddenProgramRegex {
pub regex: regex_lite::Regex,
pub reason: String,
}
#[derive(Debug, ProvidesStaticType)]
struct PolicyBuilder {
programs: RefCell<MultiMap<String, ProgramSpec>>,
forbidden_program_regexes: RefCell<Vec<ForbiddenProgramRegex>>,
forbidden_substrings: RefCell<Vec<String>>,
}
impl PolicyBuilder {
fn new() -> Self {
Self {
programs: RefCell::new(MultiMap::new()),
forbidden_program_regexes: RefCell::new(Vec::new()),
forbidden_substrings: RefCell::new(Vec::new()),
}
}
fn build(self) -> Result<Policy, regex_lite::Error> {
let programs = self.programs.into_inner();
let forbidden_program_regexes = self.forbidden_program_regexes.into_inner();
let forbidden_substrings = self.forbidden_substrings.into_inner();
Policy::new(programs, forbidden_program_regexes, forbidden_substrings)
}
fn add_program_spec(&self, program_spec: ProgramSpec) {
info!("adding program spec: {program_spec:?}");
let name = program_spec.program.clone();
let mut programs = self.programs.borrow_mut();
programs.insert(name, program_spec);
}
fn add_forbidden_substrings(&self, substrings: &[String]) {
let mut forbidden_substrings = self.forbidden_substrings.borrow_mut();
forbidden_substrings.extend_from_slice(substrings);
}
fn add_forbidden_program_regex(&self, regex: Regex, reason: String) {
let mut forbidden_program_regexes = self.forbidden_program_regexes.borrow_mut();
forbidden_program_regexes.push(ForbiddenProgramRegex { regex, reason });
}
}
#[starlark_module]
fn policy_builtins(builder: &mut GlobalsBuilder) {
fn define_program<'v>(
program: String,
system_path: Option<UnpackList<String>>,
option_bundling: Option<bool>,
combined_format: Option<bool>,
options: Option<UnpackList<Opt>>,
args: Option<UnpackList<ArgMatcher>>,
forbidden: Option<String>,
should_match: Option<UnpackList<UnpackList<String>>>,
should_not_match: Option<UnpackList<UnpackList<String>>>,
eval: &mut Evaluator,
) -> anyhow::Result<NoneType> {
let option_bundling = option_bundling.unwrap_or(false);
let system_path = system_path.map_or_else(Vec::new, |v| v.items.to_vec());
let combined_format = combined_format.unwrap_or(false);
let options = options.map_or_else(Vec::new, |v| v.items.to_vec());
let args = args.map_or_else(Vec::new, |v| v.items.to_vec());
let mut allowed_options = HashMap::<String, Opt>::new();
for opt in options {
let name = opt.name().to_string();
if allowed_options
.insert(opt.name().to_string(), opt)
.is_some()
{
return Err(anyhow::format_err!("duplicate flag: {name}"));
}
}
let program_spec = ProgramSpec::new(
program,
system_path,
option_bundling,
combined_format,
allowed_options,
args,
forbidden,
should_match
.map_or_else(Vec::new, |v| v.items.to_vec())
.into_iter()
.map(|v| v.items.to_vec())
.collect(),
should_not_match
.map_or_else(Vec::new, |v| v.items.to_vec())
.into_iter()
.map(|v| v.items.to_vec())
.collect(),
);
#[expect(clippy::unwrap_used)]
let policy_builder = eval
.extra
.as_ref()
.unwrap()
.downcast_ref::<PolicyBuilder>()
.unwrap();
policy_builder.add_program_spec(program_spec);
Ok(NoneType)
}
fn forbid_substrings(
strings: UnpackList<String>,
eval: &mut Evaluator,
) -> anyhow::Result<NoneType> {
#[expect(clippy::unwrap_used)]
let policy_builder = eval
.extra
.as_ref()
.unwrap()
.downcast_ref::<PolicyBuilder>()
.unwrap();
policy_builder.add_forbidden_substrings(&strings.items.to_vec());
Ok(NoneType)
}
fn forbid_program_regex(
regex: String,
reason: String,
eval: &mut Evaluator,
) -> anyhow::Result<NoneType> {
#[expect(clippy::unwrap_used)]
let policy_builder = eval
.extra
.as_ref()
.unwrap()
.downcast_ref::<PolicyBuilder>()
.unwrap();
let compiled_regex = regex_lite::Regex::new(&regex)?;
policy_builder.add_forbidden_program_regex(compiled_regex, reason);
Ok(NoneType)
}
fn opt(name: String, r#type: ArgMatcher, required: Option<bool>) -> anyhow::Result<Opt> {
Ok(Opt::new(
name,
OptMeta::Value(r#type.arg_type()),
required.unwrap_or(false),
))
}
fn flag(name: String) -> anyhow::Result<Opt> {
Ok(Opt::new(name, OptMeta::Flag, false))
}
}
code-rs/execpolicy/src/program.rs
-247
View File
@@ -1,247 +0,0 @@
use serde::Serialize;
use std::collections::HashMap;
use std::collections::HashSet;
use crate::ArgType;
use crate::ExecCall;
use crate::arg_matcher::ArgMatcher;
use crate::arg_resolver::PositionalArg;
use crate::arg_resolver::resolve_observed_args_with_patterns;
use crate::error::Error;
use crate::error::Result;
use crate::opt::Opt;
use crate::opt::OptMeta;
use crate::valid_exec::MatchedFlag;
use crate::valid_exec::MatchedOpt;
use crate::valid_exec::ValidExec;
#[derive(Debug)]
pub struct ProgramSpec {
pub program: String,
pub system_path: Vec<String>,
pub option_bundling: bool,
pub combined_format: bool,
pub allowed_options: HashMap<String, Opt>,
pub arg_patterns: Vec<ArgMatcher>,
forbidden: Option<String>,
required_options: HashSet<String>,
should_match: Vec<Vec<String>>,
should_not_match: Vec<Vec<String>>,
}
impl ProgramSpec {
pub fn new(
program: String,
system_path: Vec<String>,
option_bundling: bool,
combined_format: bool,
allowed_options: HashMap<String, Opt>,
arg_patterns: Vec<ArgMatcher>,
forbidden: Option<String>,
should_match: Vec<Vec<String>>,
should_not_match: Vec<Vec<String>>,
) -> Self {
let required_options = allowed_options
.iter()
.filter_map(|(name, opt)| {
if opt.required {
Some(name.clone())
} else {
None
}
})
.collect();
Self {
program,
system_path,
option_bundling,
combined_format,
allowed_options,
arg_patterns,
forbidden,
required_options,
should_match,
should_not_match,
}
}
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub enum MatchedExec {
Match { exec: ValidExec },
Forbidden { cause: Forbidden, reason: String },
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub enum Forbidden {
Program {
program: String,
exec_call: ExecCall,
},
Arg {
arg: String,
exec_call: ExecCall,
},
Exec {
exec: ValidExec,
},
}
impl ProgramSpec {
// TODO(mbolin): The idea is that there should be a set of rules defined for
// a program and the args should be checked against the rules to determine
// if the program should be allowed to run.
pub fn check(&self, exec_call: &ExecCall) -> Result<MatchedExec> {
let mut expecting_option_value: Option<(String, ArgType)> = None;
let mut args = Vec::<PositionalArg>::new();
let mut matched_flags = Vec::<MatchedFlag>::new();
let mut matched_opts = Vec::<MatchedOpt>::new();
for (index, arg) in exec_call.args.iter().enumerate() {
if let Some(expected) = expecting_option_value {
// If we are expecting an option value, then the next argument
// should be the value for the option.
// This had better not be another option!
let (name, arg_type) = expected;
if arg.starts_with("-") {
return Err(Error::OptionFollowedByOptionInsteadOfValue {
program: self.program.clone(),
option: name,
value: arg.clone(),
});
}
matched_opts.push(MatchedOpt::new(&name, arg, arg_type)?);
expecting_option_value = None;
} else if arg == "--" {
return Err(Error::DoubleDashNotSupportedYet {
program: self.program.clone(),
});
} else if arg.starts_with("-") {
match self.allowed_options.get(arg) {
Some(opt) => {
match &opt.meta {
OptMeta::Flag => {
matched_flags.push(MatchedFlag { name: arg.clone() });
// A flag does not expect an argument: continue.
continue;
}
OptMeta::Value(arg_type) => {
expecting_option_value = Some((arg.clone(), arg_type.clone()));
continue;
}
}
}
None => {
// It could be an --option=value style flag...
}
}
return Err(Error::UnknownOption {
program: self.program.clone(),
option: arg.clone(),
});
} else {
args.push(PositionalArg {
index,
value: arg.clone(),
});
}
}
if let Some(expected) = expecting_option_value {
let (name, _arg_type) = expected;
return Err(Error::OptionMissingValue {
program: self.program.clone(),
option: name,
});
}
let matched_args =
resolve_observed_args_with_patterns(&self.program, args, &self.arg_patterns)?;
// Verify all required options are present.
let matched_opt_names: HashSet<String> = matched_opts
.iter()
.map(|opt| opt.name().to_string())
.collect();
if !matched_opt_names.is_superset(&self.required_options) {
let mut options = self
.required_options
.difference(&matched_opt_names)
.map(String::from)
.collect::<Vec<_>>();
options.sort();
return Err(Error::MissingRequiredOptions {
program: self.program.clone(),
options,
});
}
let exec = ValidExec {
program: self.program.clone(),
flags: matched_flags,
opts: matched_opts,
args: matched_args,
system_path: self.system_path.clone(),
};
match &self.forbidden {
Some(reason) => Ok(MatchedExec::Forbidden {
cause: Forbidden::Exec { exec },
reason: reason.clone(),
}),
None => Ok(MatchedExec::Match { exec }),
}
}
pub fn verify_should_match_list(&self) -> Vec<PositiveExampleFailedCheck> {
let mut violations = Vec::new();
for good in &self.should_match {
let exec_call = ExecCall {
program: self.program.clone(),
args: good.clone(),
};
match self.check(&exec_call) {
Ok(_) => {}
Err(error) => {
violations.push(PositiveExampleFailedCheck {
program: self.program.clone(),
args: good.clone(),
error,
});
}
}
}
violations
}
pub fn verify_should_not_match_list(&self) -> Vec<NegativeExamplePassedCheck> {
let mut violations = Vec::new();
for bad in &self.should_not_match {
let exec_call = ExecCall {
program: self.program.clone(),
args: bad.clone(),
};
if self.check(&exec_call).is_ok() {
violations.push(NegativeExamplePassedCheck {
program: self.program.clone(),
args: bad.clone(),
});
}
}
violations
}
}
#[derive(Debug, Eq, PartialEq)]
pub struct PositiveExampleFailedCheck {
pub program: String,
pub args: Vec<String>,
pub error: Error,
}
#[derive(Debug, Eq, PartialEq)]
pub struct NegativeExamplePassedCheck {
pub program: String,
pub args: Vec<String>,
}
code-rs/execpolicy/src/sed_command.rs
-17
View File
@@ -1,17 +0,0 @@
use crate::error::Error;
use crate::error::Result;
pub fn parse_sed_command(sed_command: &str) -> Result<()> {
// For now, we parse only commands like `122,202p`.
if let Some(stripped) = sed_command.strip_suffix("p")
&& let Some((first, rest)) = stripped.split_once(",")
&& first.parse::<u64>().is_ok()
&& rest.parse::<u64>().is_ok()
{
return Ok(());
}
Err(Error::SedCommandNotProvablySafe {
command: sed_command.to_string(),
})
}
code-rs/execpolicy/src/valid_exec.rs
-95
View File
@@ -1,95 +0,0 @@
use crate::arg_type::ArgType;
use crate::error::Result;
use serde::Serialize;
/// exec() invocation that has been accepted by a `Policy`.
#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)]
pub struct ValidExec {
pub program: String,
pub flags: Vec<MatchedFlag>,
pub opts: Vec<MatchedOpt>,
pub args: Vec<MatchedArg>,
/// If non-empty, a prioritized list of paths to try instead of `program`.
/// For example, `/bin/ls` is harder to compromise than whatever `ls`
/// happens to be in the user's `$PATH`, so `/bin/ls` would be included for
/// `ls`. The caller is free to disregard this list and use `program`.
pub system_path: Vec<String>,
}
impl ValidExec {
pub fn new(program: &str, args: Vec<MatchedArg>, system_path: &[&str]) -> Self {
Self {
program: program.to_string(),
flags: vec![],
opts: vec![],
args,
system_path: system_path.iter().map(|&s| s.to_string()).collect(),
}
}
/// Whether a possible side effect of running this command includes writing
/// a file.
pub fn might_write_files(&self) -> bool {
self.opts.iter().any(|opt| opt.r#type.might_write_file())
|| self.args.iter().any(|opt| opt.r#type.might_write_file())
}
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct MatchedArg {
pub index: usize,
pub r#type: ArgType,
pub value: String,
}
impl MatchedArg {
pub fn new(index: usize, r#type: ArgType, value: &str) -> Result<Self> {
r#type.validate(value)?;
Ok(Self {
index,
r#type,
value: value.to_string(),
})
}
}
/// A match for an option declared with opt() in a .policy file.
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct MatchedOpt {
/// Name of the option that was matched.
pub name: String,
/// Value supplied for the option.
pub value: String,
/// Type of the value supplied for the option.
pub r#type: ArgType,
}
impl MatchedOpt {
pub fn new(name: &str, value: &str, r#type: ArgType) -> Result<Self> {
r#type.validate(value)?;
Ok(Self {
name: name.to_string(),
value: value.to_string(),
r#type,
})
}
pub fn name(&self) -> &str {
&self.name
}
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct MatchedFlag {
/// Name of the flag that was matched.
pub name: String,
}
impl MatchedFlag {
pub fn new(name: &str) -> Self {
Self {
name: name.to_string(),
}
}
}
code-rs/execpolicy/tests/all.rs
-3
View File
@@ -1,3 +0,0 @@
// Single integration test binary that aggregates all test modules.
// The submodules live in `tests/suite/`.
mod suite;
code-rs/execpolicy/tests/suite/bad.rs
-9
View File
@@ -1,9 +0,0 @@
use code_execpolicy::NegativeExamplePassedCheck;
use code_execpolicy::get_default_policy;
#[test]
fn verify_everything_in_bad_list_is_rejected() {
let policy = get_default_policy().expect("failed to load default policy");
let violations = policy.check_each_bad_list_individually();
assert_eq!(Vec::<NegativeExamplePassedCheck>::new(), violations);
}
code-rs/execpolicy/tests/suite/cp.rs
-86
View File
@@ -1,86 +0,0 @@
extern crate code_execpolicy;
use code_execpolicy::ArgMatcher;
use code_execpolicy::ArgType;
use code_execpolicy::Error;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedArg;
use code_execpolicy::MatchedExec;
use code_execpolicy::Policy;
use code_execpolicy::Result;
use code_execpolicy::ValidExec;
use code_execpolicy::get_default_policy;
#[expect(clippy::expect_used)]
fn setup() -> Policy {
get_default_policy().expect("failed to load default policy")
}
#[test]
fn test_cp_no_args() {
let policy = setup();
let cp = ExecCall::new("cp", &[]);
assert_eq!(
Err(Error::NotEnoughArgs {
program: "cp".to_string(),
args: vec![],
arg_patterns: vec![ArgMatcher::ReadableFiles, ArgMatcher::WriteableFile]
}),
policy.check(&cp)
)
}
#[test]
fn test_cp_one_arg() {
let policy = setup();
let cp = ExecCall::new("cp", &["foo/bar"]);
assert_eq!(
Err(Error::VarargMatcherDidNotMatchAnything {
program: "cp".to_string(),
matcher: ArgMatcher::ReadableFiles,
}),
policy.check(&cp)
);
}
#[test]
fn test_cp_one_file() -> Result<()> {
let policy = setup();
let cp = ExecCall::new("cp", &["foo/bar", "../baz"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new(
"cp",
vec![
MatchedArg::new(0, ArgType::ReadableFile, "foo/bar")?,
MatchedArg::new(1, ArgType::WriteableFile, "../baz")?,
],
&["/bin/cp", "/usr/bin/cp"]
)
}),
policy.check(&cp)
);
Ok(())
}
#[test]
fn test_cp_multiple_files() -> Result<()> {
let policy = setup();
let cp = ExecCall::new("cp", &["foo", "bar", "baz"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new(
"cp",
vec![
MatchedArg::new(0, ArgType::ReadableFile, "foo")?,
MatchedArg::new(1, ArgType::ReadableFile, "bar")?,
MatchedArg::new(2, ArgType::WriteableFile, "baz")?,
],
&["/bin/cp", "/usr/bin/cp"]
)
}),
policy.check(&cp)
);
Ok(())
}
code-rs/execpolicy/tests/suite/good.rs
-9
View File
@@ -1,9 +0,0 @@
use code_execpolicy::PositiveExampleFailedCheck;
use code_execpolicy::get_default_policy;
#[test]
fn verify_everything_in_good_list_is_allowed() {
let policy = get_default_policy().expect("failed to load default policy");
let violations = policy.check_each_good_list_individually();
assert_eq!(Vec::<PositiveExampleFailedCheck>::new(), violations);
}
code-rs/execpolicy/tests/suite/head.rs
-136
View File
@@ -1,136 +0,0 @@
use code_execpolicy::ArgMatcher;
use code_execpolicy::ArgType;
use code_execpolicy::Error;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedArg;
use code_execpolicy::MatchedExec;
use code_execpolicy::MatchedOpt;
use code_execpolicy::Policy;
use code_execpolicy::Result;
use code_execpolicy::ValidExec;
use code_execpolicy::get_default_policy;
extern crate code_execpolicy;
#[expect(clippy::expect_used)]
fn setup() -> Policy {
get_default_policy().expect("failed to load default policy")
}
#[test]
fn test_head_no_args() {
let policy = setup();
let head = ExecCall::new("head", &[]);
// It is actually valid to call `head` without arguments: it will read from
// stdin instead of from a file. Though recall that a command rejected by
// the policy is not "unsafe:" it just means that this library cannot
// *guarantee* that the command is safe.
//
// If we start verifying individual components of a shell command, such as:
// `find . -name | head -n 10`, then it might be important to allow the
// no-arg case.
assert_eq!(
Err(Error::VarargMatcherDidNotMatchAnything {
program: "head".to_string(),
matcher: ArgMatcher::ReadableFiles,
}),
policy.check(&head)
)
}
#[test]
fn test_head_one_file_no_flags() -> Result<()> {
let policy = setup();
let head = ExecCall::new("head", &["src/extension.ts"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new(
"head",
vec![MatchedArg::new(
0,
ArgType::ReadableFile,
"src/extension.ts"
)?],
&["/bin/head", "/usr/bin/head"]
)
}),
policy.check(&head)
);
Ok(())
}
#[test]
fn test_head_one_flag_one_file() -> Result<()> {
let policy = setup();
let head = ExecCall::new("head", &["-n", "100", "src/extension.ts"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "head".to_string(),
flags: vec![],
opts: vec![
MatchedOpt::new("-n", "100", ArgType::PositiveInteger)
.expect("should validate")
],
args: vec![MatchedArg::new(
2,
ArgType::ReadableFile,
"src/extension.ts"
)?],
system_path: vec!["/bin/head".to_string(), "/usr/bin/head".to_string()],
}
}),
policy.check(&head)
);
Ok(())
}
#[test]
fn test_head_invalid_n_as_0() {
let policy = setup();
let head = ExecCall::new("head", &["-n", "0", "src/extension.ts"]);
assert_eq!(
Err(Error::InvalidPositiveInteger {
value: "0".to_string(),
}),
policy.check(&head)
)
}
#[test]
fn test_head_invalid_n_as_nonint_float() {
let policy = setup();
let head = ExecCall::new("head", &["-n", "1.5", "src/extension.ts"]);
assert_eq!(
Err(Error::InvalidPositiveInteger {
value: "1.5".to_string(),
}),
policy.check(&head)
)
}
#[test]
fn test_head_invalid_n_as_float() {
let policy = setup();
let head = ExecCall::new("head", &["-n", "1.0", "src/extension.ts"]);
assert_eq!(
Err(Error::InvalidPositiveInteger {
value: "1.0".to_string(),
}),
policy.check(&head)
)
}
#[test]
fn test_head_invalid_n_as_negative_int() {
let policy = setup();
let head = ExecCall::new("head", &["-n", "-1", "src/extension.ts"]);
assert_eq!(
Err(Error::OptionFollowedByOptionInsteadOfValue {
program: "head".to_string(),
option: "-n".to_string(),
value: "-1".to_string(),
}),
policy.check(&head)
)
}
code-rs/execpolicy/tests/suite/literal.rs
-50
View File
@@ -1,50 +0,0 @@
use code_execpolicy::ArgType;
use code_execpolicy::Error;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedArg;
use code_execpolicy::MatchedExec;
use code_execpolicy::PolicyParser;
use code_execpolicy::Result;
use code_execpolicy::ValidExec;
extern crate code_execpolicy;
#[test]
fn test_invalid_subcommand() -> Result<()> {
let unparsed_policy = r#"
define_program(
program="fake_executable",
args=["subcommand", "sub-subcommand"],
)
"#;
let parser = PolicyParser::new("test_invalid_subcommand", unparsed_policy);
let policy = parser.parse().expect("failed to parse policy");
let valid_call = ExecCall::new("fake_executable", &["subcommand", "sub-subcommand"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new(
"fake_executable",
vec![
MatchedArg::new(0, ArgType::Literal("subcommand".to_string()), "subcommand")?,
MatchedArg::new(
1,
ArgType::Literal("sub-subcommand".to_string()),
"sub-subcommand"
)?,
],
&[]
)
}),
policy.check(&valid_call)
);
let invalid_call = ExecCall::new("fake_executable", &["subcommand", "not-a-real-subcommand"]);
assert_eq!(
Err(Error::LiteralValueDidNotMatch {
expected: "sub-subcommand".to_string(),
actual: "not-a-real-subcommand".to_string()
}),
policy.check(&invalid_call)
);
Ok(())
}
code-rs/execpolicy/tests/suite/ls.rs
-167
View File
@@ -1,167 +0,0 @@
extern crate code_execpolicy;
use code_execpolicy::ArgType;
use code_execpolicy::Error;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedArg;
use code_execpolicy::MatchedExec;
use code_execpolicy::MatchedFlag;
use code_execpolicy::Policy;
use code_execpolicy::Result;
use code_execpolicy::ValidExec;
use code_execpolicy::get_default_policy;
#[expect(clippy::expect_used)]
fn setup() -> Policy {
get_default_policy().expect("failed to load default policy")
}
#[test]
fn test_ls_no_args() {
let policy = setup();
let ls = ExecCall::new("ls", &[]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new("ls", vec![], &["/bin/ls", "/usr/bin/ls"])
}),
policy.check(&ls)
);
}
#[test]
fn test_ls_dash_a_dash_l() {
let policy = setup();
let args = &["-a", "-l"];
let ls_a_l = ExecCall::new("ls", args);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "ls".into(),
flags: vec![MatchedFlag::new("-a"), MatchedFlag::new("-l")],
system_path: ["/bin/ls".into(), "/usr/bin/ls".into()].into(),
..Default::default()
}
}),
policy.check(&ls_a_l)
);
}
#[test]
fn test_ls_dash_z() {
let policy = setup();
// -z is currently an invalid option for ls, but it has so many options,
// perhaps it will get added at some point...
let ls_z = ExecCall::new("ls", &["-z"]);
assert_eq!(
Err(Error::UnknownOption {
program: "ls".into(),
option: "-z".into()
}),
policy.check(&ls_z)
);
}
#[test]
fn test_ls_dash_al() {
let policy = setup();
// This currently fails, but it should pass once option_bundling=True is implemented.
let ls_al = ExecCall::new("ls", &["-al"]);
assert_eq!(
Err(Error::UnknownOption {
program: "ls".into(),
option: "-al".into()
}),
policy.check(&ls_al)
);
}
#[test]
fn test_ls_one_file_arg() -> Result<()> {
let policy = setup();
let ls_one_file_arg = ExecCall::new("ls", &["foo"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new(
"ls",
vec![MatchedArg::new(0, ArgType::ReadableFile, "foo")?],
&["/bin/ls", "/usr/bin/ls"]
)
}),
policy.check(&ls_one_file_arg)
);
Ok(())
}
#[test]
fn test_ls_multiple_file_args() -> Result<()> {
let policy = setup();
let ls_multiple_file_args = ExecCall::new("ls", &["foo", "bar", "baz"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec::new(
"ls",
vec![
MatchedArg::new(0, ArgType::ReadableFile, "foo")?,
MatchedArg::new(1, ArgType::ReadableFile, "bar")?,
MatchedArg::new(2, ArgType::ReadableFile, "baz")?,
],
&["/bin/ls", "/usr/bin/ls"]
)
}),
policy.check(&ls_multiple_file_args)
);
Ok(())
}
#[test]
fn test_ls_multiple_flags_and_file_args() -> Result<()> {
let policy = setup();
let ls_multiple_flags_and_file_args = ExecCall::new("ls", &["-l", "-a", "foo", "bar", "baz"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "ls".into(),
flags: vec![MatchedFlag::new("-l"), MatchedFlag::new("-a")],
args: vec![
MatchedArg::new(2, ArgType::ReadableFile, "foo")?,
MatchedArg::new(3, ArgType::ReadableFile, "bar")?,
MatchedArg::new(4, ArgType::ReadableFile, "baz")?,
],
system_path: ["/bin/ls".into(), "/usr/bin/ls".into()].into(),
..Default::default()
}
}),
policy.check(&ls_multiple_flags_and_file_args)
);
Ok(())
}
#[test]
fn test_flags_after_file_args() -> Result<()> {
let policy = setup();
// TODO(mbolin): While this is "safe" in that it will not do anything bad
// to the user's machine, it will fail because apparently `ls` does not
// allow flags after file arguments (as some commands do). We should
// extend define_program() to make this part of the configuration so that
// this command is disallowed.
let ls_flags_after_file_args = ExecCall::new("ls", &["foo", "-l"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "ls".into(),
flags: vec![MatchedFlag::new("-l")],
args: vec![MatchedArg::new(0, ArgType::ReadableFile, "foo")?],
system_path: ["/bin/ls".into(), "/usr/bin/ls".into()].into(),
..Default::default()
}
}),
policy.check(&ls_flags_after_file_args)
);
Ok(())
}
code-rs/execpolicy/tests/suite/mod.rs
-10
View File
@@ -1,10 +0,0 @@
// Aggregates all former standalone integration tests as modules.
mod bad;
mod cp;
mod good;
mod head;
mod literal;
mod ls;
mod parse_sed_command;
mod pwd;
mod sed;
code-rs/execpolicy/tests/suite/parse_sed_command.rs
-23
View File
@@ -1,23 +0,0 @@
use code_execpolicy::Error;
use code_execpolicy::parse_sed_command;
#[test]
fn parses_simple_print_command() {
assert_eq!(parse_sed_command("122,202p"), Ok(()));
}
#[test]
fn rejects_malformed_print_command() {
assert_eq!(
parse_sed_command("122,202"),
Err(Error::SedCommandNotProvablySafe {
command: "122,202".to_string(),
})
);
assert_eq!(
parse_sed_command("122202"),
Err(Error::SedCommandNotProvablySafe {
command: "122202".to_string(),
})
);
}
code-rs/execpolicy/tests/suite/pwd.rs
-86
View File
@@ -1,86 +0,0 @@
extern crate code_execpolicy;
use std::vec;
use code_execpolicy::Error;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedExec;
use code_execpolicy::MatchedFlag;
use code_execpolicy::Policy;
use code_execpolicy::PositionalArg;
use code_execpolicy::ValidExec;
use code_execpolicy::get_default_policy;
#[expect(clippy::expect_used)]
fn setup() -> Policy {
get_default_policy().expect("failed to load default policy")
}
#[test]
fn test_pwd_no_args() {
let policy = setup();
let pwd = ExecCall::new("pwd", &[]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "pwd".into(),
..Default::default()
}
}),
policy.check(&pwd)
);
}
#[test]
fn test_pwd_capital_l() {
let policy = setup();
let pwd = ExecCall::new("pwd", &["-L"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "pwd".into(),
flags: vec![MatchedFlag::new("-L")],
..Default::default()
}
}),
policy.check(&pwd)
);
}
#[test]
fn test_pwd_capital_p() {
let policy = setup();
let pwd = ExecCall::new("pwd", &["-P"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "pwd".into(),
flags: vec![MatchedFlag::new("-P")],
..Default::default()
}
}),
policy.check(&pwd)
);
}
#[test]
fn test_pwd_extra_args() {
let policy = setup();
let pwd = ExecCall::new("pwd", &["foo", "bar"]);
assert_eq!(
Err(Error::UnexpectedArguments {
program: "pwd".to_string(),
args: vec![
PositionalArg {
index: 0,
value: "foo".to_string()
},
PositionalArg {
index: 1,
value: "bar".to_string()
},
],
}),
policy.check(&pwd)
);
}
code-rs/execpolicy/tests/suite/sed.rs
-87
View File
@@ -1,87 +0,0 @@
extern crate code_execpolicy;
use code_execpolicy::ArgType;
use code_execpolicy::Error;
use code_execpolicy::ExecCall;
use code_execpolicy::MatchedArg;
use code_execpolicy::MatchedExec;
use code_execpolicy::MatchedFlag;
use code_execpolicy::MatchedOpt;
use code_execpolicy::Policy;
use code_execpolicy::Result;
use code_execpolicy::ValidExec;
use code_execpolicy::get_default_policy;
#[expect(clippy::expect_used)]
fn setup() -> Policy {
get_default_policy().expect("failed to load default policy")
}
#[test]
fn test_sed_print_specific_lines() -> Result<()> {
let policy = setup();
let sed = ExecCall::new("sed", &["-n", "122,202p", "hello.txt"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "sed".to_string(),
flags: vec![MatchedFlag::new("-n")],
args: vec![
MatchedArg::new(1, ArgType::SedCommand, "122,202p")?,
MatchedArg::new(2, ArgType::ReadableFile, "hello.txt")?,
],
system_path: vec!["/usr/bin/sed".to_string()],
..Default::default()
}
}),
policy.check(&sed)
);
Ok(())
}
#[test]
fn test_sed_print_specific_lines_with_e_flag() -> Result<()> {
let policy = setup();
let sed = ExecCall::new("sed", &["-n", "-e", "122,202p", "hello.txt"]);
assert_eq!(
Ok(MatchedExec::Match {
exec: ValidExec {
program: "sed".to_string(),
flags: vec![MatchedFlag::new("-n")],
opts: vec![
MatchedOpt::new("-e", "122,202p", ArgType::SedCommand)
.expect("should validate")
],
args: vec![MatchedArg::new(3, ArgType::ReadableFile, "hello.txt")?],
system_path: vec!["/usr/bin/sed".to_string()],
}
}),
policy.check(&sed)
);
Ok(())
}
#[test]
fn test_sed_reject_dangerous_command() {
let policy = setup();
let sed = ExecCall::new("sed", &["-e", "s/y/echo hi/e", "hello.txt"]);
assert_eq!(
Err(Error::SedCommandNotProvablySafe {
command: "s/y/echo hi/e".to_string(),
}),
policy.check(&sed)
);
}
#[test]
fn test_sed_verify_e_or_pattern_is_required() {
let policy = setup();
let sed = ExecCall::new("sed", &["122,202p"]);
assert_eq!(
Err(Error::MissingRequiredOptions {
program: "sed".to_string(),
options: vec!["-e".to_string()],
}),
policy.check(&sed)
);
}
code-rs/git-apply/Cargo.toml
+1 -4
View File
@@ -11,7 +11,4 @@ path = "src/lib.rs"
workspace = true
[dependencies]
once_cell = "1"
regex = "1"
tempfile = "3"
codex-git-apply = { workspace = true }
code-rs/git-apply/src/lib.rs
+2 -698
View File
@@ -1,698 +1,2 @@
use once_cell::sync::Lazy;
use regex::Regex;
use std::ffi::OsStr;
use std::io;
use std::path::Path;
use std::path::PathBuf;
#[derive(Debug, Clone)]
pub struct ApplyGitRequest {
pub cwd: PathBuf,
pub diff: String,
pub revert: bool,
pub preflight: bool,
}
#[derive(Debug, Clone)]
pub struct ApplyGitResult {
pub exit_code: i32,
pub applied_paths: Vec<String>,
pub skipped_paths: Vec<String>,
pub conflicted_paths: Vec<String>,
pub stdout: String,
pub stderr: String,
pub cmd_for_log: String,
}
pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
let git_root = resolve_git_root(&req.cwd)?;
// Write unified diff into a temporary file
let (tmpdir, patch_path) = write_temp_patch(&req.diff)?;
// Keep tmpdir alive until function end to ensure the file exists
let _guard = tmpdir;
if req.revert && !req.preflight {
// Stage WT paths first to avoid index mismatch on revert.
stage_paths(&git_root, &req.diff)?;
}
// Build git args
let mut args: Vec<String> = vec!["apply".into(), "--3way".into()];
if req.revert {
args.push("-R".into());
}
// Optional: additional git config via env knob (defaults OFF)
let mut cfg_parts: Vec<String> = Vec::new();
if let Ok(cfg) = std::env::var("CODEX_APPLY_GIT_CFG") {
for pair in cfg.split(',') {
let p = pair.trim();
if p.is_empty() || !p.contains('=') {
continue;
}
cfg_parts.push("-c".into());
cfg_parts.push(p.to_string());
}
}
args.push(patch_path.to_string_lossy().to_string());
// Optional preflight: dry-run only; do not modify working tree
if req.preflight {
let mut check_args = vec!["apply".to_string(), "--check".to_string()];
if req.revert {
check_args.push("-R".to_string());
}
check_args.push(patch_path.to_string_lossy().to_string());
let rendered = render_command_for_log(&git_root, &cfg_parts, &check_args);
let (c_code, c_out, c_err) = run_git(&git_root, &cfg_parts, &check_args)?;
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
parse_git_apply_output(&c_out, &c_err);
applied_paths.sort();
applied_paths.dedup();
skipped_paths.sort();
skipped_paths.dedup();
conflicted_paths.sort();
conflicted_paths.dedup();
return Ok(ApplyGitResult {
exit_code: c_code,
applied_paths,
skipped_paths,
conflicted_paths,
stdout: c_out,
stderr: c_err,
cmd_for_log: rendered,
});
}
let cmd_for_log = render_command_for_log(&git_root, &cfg_parts, &args);
let (code, stdout, stderr) = run_git(&git_root, &cfg_parts, &args)?;
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
parse_git_apply_output(&stdout, &stderr);
applied_paths.sort();
applied_paths.dedup();
skipped_paths.sort();
skipped_paths.dedup();
conflicted_paths.sort();
conflicted_paths.dedup();
Ok(ApplyGitResult {
exit_code: code,
applied_paths,
skipped_paths,
conflicted_paths,
stdout,
stderr,
cmd_for_log,
})
}
fn resolve_git_root(cwd: &Path) -> io::Result<PathBuf> {
let out = std::process::Command::new("git")
.arg("rev-parse")
.arg("--show-toplevel")
.current_dir(cwd)
.output()?;
let code = out.status.code().unwrap_or(-1);
if code != 0 {
return Err(io::Error::other(format!(
"not a git repository (exit {}): {}",
code,
String::from_utf8_lossy(&out.stderr)
)));
}
let root = String::from_utf8_lossy(&out.stdout).trim().to_string();
Ok(PathBuf::from(root))
}
fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, PathBuf)> {
let dir = tempfile::tempdir()?;
let path = dir.path().join("patch.diff");
std::fs::write(&path, diff)?;
Ok((dir, path))
}
fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, String, String)> {
let mut cmd = std::process::Command::new("git");
for p in git_cfg {
cmd.arg(p);
}
for a in args {
cmd.arg(a);
}
let out = cmd.current_dir(cwd).output()?;
let code = out.status.code().unwrap_or(-1);
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
let stderr = String::from_utf8_lossy(&out.stderr).into_owned();
Ok((code, stdout, stderr))
}
fn quote_shell(s: &str) -> String {
let simple = s
.chars()
.all(|c| c.is_ascii_alphanumeric() || "-_.:/@%+".contains(c));
if simple {
s.to_string()
} else {
format!("'{}'", s.replace('\'', "'\\''"))
}
}
fn render_command_for_log(cwd: &Path, git_cfg: &[String], args: &[String]) -> String {
let mut parts: Vec<String> = Vec::new();
parts.push("git".to_string());
for a in git_cfg {
parts.push(quote_shell(a));
}
for a in args {
parts.push(quote_shell(a));
}
format!(
"(cd {} && {})",
quote_shell(&cwd.display().to_string()),
parts.join(" ")
)
}
pub fn extract_paths_from_patch(diff_text: &str) -> Vec<String> {
static RE: Lazy<Regex> = Lazy::new(|| {
Regex::new(r"(?m)^diff --git a/(.*?) b/(.*)$")
.unwrap_or_else(|e| panic!("invalid regex: {e}"))
});
let mut set = std::collections::BTreeSet::new();
for caps in RE.captures_iter(diff_text) {
if let Some(a) = caps.get(1).map(|m| m.as_str())
&& a != "/dev/null"
&& !a.trim().is_empty()
{
set.insert(a.to_string());
}
if let Some(b) = caps.get(2).map(|m| m.as_str())
&& b != "/dev/null"
&& !b.trim().is_empty()
{
set.insert(b.to_string());
}
}
set.into_iter().collect()
}
pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
let paths = extract_paths_from_patch(diff);
let mut existing: Vec<String> = Vec::new();
for p in paths {
let joined = git_root.join(&p);
if std::fs::symlink_metadata(&joined).is_ok() {
existing.push(p);
}
}
if existing.is_empty() {
return Ok(());
}
let mut cmd = std::process::Command::new("git");
cmd.arg("add");
cmd.arg("--");
for p in &existing {
cmd.arg(OsStr::new(p));
}
let out = cmd.current_dir(git_root).output()?;
let _code = out.status.code().unwrap_or(-1);
// We do not hard fail staging; best-effort is OK. Return Ok even on non-zero.
Ok(())
}
// ============ Parser ported from VS Code (TS) ============
pub fn parse_git_apply_output(
stdout: &str,
stderr: &str,
) -> (Vec<String>, Vec<String>, Vec<String>) {
let combined = [stdout, stderr]
.iter()
.filter(|s| !s.is_empty())
.cloned()
.collect::<Vec<&str>>()
.join("\n");
let mut applied = std::collections::BTreeSet::new();
let mut skipped = std::collections::BTreeSet::new();
let mut conflicted = std::collections::BTreeSet::new();
let mut last_seen_path: Option<String> = None;
fn add(set: &mut std::collections::BTreeSet<String>, raw: &str) {
let trimmed = raw.trim();
if trimmed.is_empty() {
return;
}
let first = trimmed.chars().next().unwrap_or('\0');
let last = trimmed.chars().last().unwrap_or('\0');
let unquoted = if (first == '"' || first == '\'') && last == first && trimmed.len() >= 2 {
&trimmed[1..trimmed.len() - 1]
} else {
trimmed
};
if !unquoted.is_empty() {
set.insert(unquoted.to_string());
}
}
static APPLIED_CLEAN: Lazy<Regex> =
Lazy::new(|| regex_ci("^Applied patch(?: to)?\\s+(?P<path>.+?)\\s+cleanly\\.?$"));
static APPLIED_CONFLICTS: Lazy<Regex> =
Lazy::new(|| regex_ci("^Applied patch(?: to)?\\s+(?P<path>.+?)\\s+with conflicts\\.?$"));
static APPLYING_WITH_REJECTS: Lazy<Regex> = Lazy::new(|| {
regex_ci("^Applying patch\\s+(?P<path>.+?)\\s+with\\s+\\d+\\s+rejects?\\.{0,3}$")
});
static CHECKING_PATCH: Lazy<Regex> =
Lazy::new(|| regex_ci("^Checking patch\\s+(?P<path>.+?)\\.\\.\\.$"));
static UNMERGED_LINE: Lazy<Regex> = Lazy::new(|| regex_ci("^U\\s+(?P<path>.+)$"));
static PATCH_FAILED: Lazy<Regex> =
Lazy::new(|| regex_ci("^error:\\s+patch failed:\\s+(?P<path>.+?)(?::\\d+)?(?:\\s|$)"));
static DOES_NOT_APPLY: Lazy<Regex> =
Lazy::new(|| regex_ci("^error:\\s+(?P<path>.+?):\\s+patch does not apply$"));
static THREE_WAY_START: Lazy<Regex> = Lazy::new(|| {
regex_ci("^(?:Performing three-way merge|Falling back to three-way merge)\\.\\.\\.$")
});
static THREE_WAY_FAILED: Lazy<Regex> =
Lazy::new(|| regex_ci("^Failed to perform three-way merge\\.\\.\\.$"));
static FALLBACK_DIRECT: Lazy<Regex> =
Lazy::new(|| regex_ci("^Falling back to direct application\\.\\.\\.$"));
static LACKS_BLOB: Lazy<Regex> = Lazy::new(|| {
regex_ci(
"^(?:error: )?repository lacks the necessary blob to (?:perform|fall back on) 3-?way merge\\.?$",
)
});
static INDEX_MISMATCH: Lazy<Regex> =
Lazy::new(|| regex_ci("^error:\\s+(?P<path>.+?):\\s+does not match index\\b"));
static NOT_IN_INDEX: Lazy<Regex> =
Lazy::new(|| regex_ci("^error:\\s+(?P<path>.+?):\\s+does not exist in index\\b"));
static ALREADY_EXISTS_WT: Lazy<Regex> = Lazy::new(|| {
regex_ci("^error:\\s+(?P<path>.+?)\\s+already exists in (?:the )?working directory\\b")
});
static FILE_EXISTS: Lazy<Regex> =
Lazy::new(|| regex_ci("^error:\\s+patch failed:\\s+(?P<path>.+?)\\s+File exists"));
static RENAMED_DELETED: Lazy<Regex> =
Lazy::new(|| regex_ci("^error:\\s+path\\s+(?P<path>.+?)\\s+has been renamed\\/deleted"));
static CANNOT_APPLY_BINARY: Lazy<Regex> = Lazy::new(|| {
regex_ci(
"^error:\\s+cannot apply binary patch to\\s+['\\\"]?(?P<path>.+?)['\\\"]?\\s+without full index line$",
)
});
static BINARY_DOES_NOT_APPLY: Lazy<Regex> = Lazy::new(|| {
regex_ci("^error:\\s+binary patch does not apply to\\s+['\\\"]?(?P<path>.+?)['\\\"]?$")
});
static BINARY_INCORRECT_RESULT: Lazy<Regex> = Lazy::new(|| {
regex_ci(
"^error:\\s+binary patch to\\s+['\\\"]?(?P<path>.+?)['\\\"]?\\s+creates incorrect result\\b",
)
});
static CANNOT_READ_CURRENT: Lazy<Regex> = Lazy::new(|| {
regex_ci("^error:\\s+cannot read the current contents of\\s+['\\\"]?(?P<path>.+?)['\\\"]?$")
});
static SKIPPED_PATCH: Lazy<Regex> =
Lazy::new(|| regex_ci("^Skipped patch\\s+['\\\"]?(?P<path>.+?)['\\\"]\\.$"));
static CANNOT_MERGE_BINARY_WARN: Lazy<Regex> = Lazy::new(|| {
regex_ci(
"^warning:\\s*Cannot merge binary files:\\s+(?P<path>.+?)\\s+\\(ours\\s+vs\\.\\s+theirs\\)",
)
});
for raw_line in combined.lines() {
let line = raw_line.trim();
if line.is_empty() {
continue;
}
// === "Checking patch <path>..." tracking ===
if let Some(c) = CHECKING_PATCH.captures(line) {
if let Some(m) = c.name("path") {
last_seen_path = Some(m.as_str().to_string());
}
continue;
}
// === Status lines ===
if let Some(c) = APPLIED_CLEAN.captures(line) {
if let Some(m) = c.name("path") {
add(&mut applied, m.as_str());
let p = applied.iter().next_back().cloned();
if let Some(p) = p {
conflicted.remove(&p);
skipped.remove(&p);
last_seen_path = Some(p);
}
}
continue;
}
if let Some(c) = APPLIED_CONFLICTS.captures(line) {
if let Some(m) = c.name("path") {
add(&mut conflicted, m.as_str());
let p = conflicted.iter().next_back().cloned();
if let Some(p) = p {
applied.remove(&p);
skipped.remove(&p);
last_seen_path = Some(p);
}
}
continue;
}
if let Some(c) = APPLYING_WITH_REJECTS.captures(line) {
if let Some(m) = c.name("path") {
add(&mut conflicted, m.as_str());
let p = conflicted.iter().next_back().cloned();
if let Some(p) = p {
applied.remove(&p);
skipped.remove(&p);
last_seen_path = Some(p);
}
}
continue;
}
// === “U <path>” after conflicts ===
if let Some(c) = UNMERGED_LINE.captures(line) {
if let Some(m) = c.name("path") {
add(&mut conflicted, m.as_str());
let p = conflicted.iter().next_back().cloned();
if let Some(p) = p {
applied.remove(&p);
skipped.remove(&p);
last_seen_path = Some(p);
}
}
continue;
}
// === Early hints ===
if PATCH_FAILED.is_match(line) || DOES_NOT_APPLY.is_match(line) {
if let Some(c) = PATCH_FAILED
.captures(line)
.or_else(|| DOES_NOT_APPLY.captures(line))
&& let Some(m) = c.name("path")
{
add(&mut skipped, m.as_str());
last_seen_path = Some(m.as_str().to_string());
}
continue;
}
// === Ignore narration ===
if THREE_WAY_START.is_match(line) || FALLBACK_DIRECT.is_match(line) {
continue;
}
// === 3-way failed entirely; attribute to last_seen_path ===
if THREE_WAY_FAILED.is_match(line) || LACKS_BLOB.is_match(line) {
if let Some(p) = last_seen_path.clone() {
add(&mut skipped, &p);
applied.remove(&p);
conflicted.remove(&p);
}
continue;
}
// === Skips / I/O problems ===
if let Some(c) = INDEX_MISMATCH
.captures(line)
.or_else(|| NOT_IN_INDEX.captures(line))
.or_else(|| ALREADY_EXISTS_WT.captures(line))
.or_else(|| FILE_EXISTS.captures(line))
.or_else(|| RENAMED_DELETED.captures(line))
.or_else(|| CANNOT_APPLY_BINARY.captures(line))
.or_else(|| BINARY_DOES_NOT_APPLY.captures(line))
.or_else(|| BINARY_INCORRECT_RESULT.captures(line))
.or_else(|| CANNOT_READ_CURRENT.captures(line))
.or_else(|| SKIPPED_PATCH.captures(line))
{
if let Some(m) = c.name("path") {
add(&mut skipped, m.as_str());
let p_now = skipped.iter().next_back().cloned();
if let Some(p) = p_now {
applied.remove(&p);
conflicted.remove(&p);
last_seen_path = Some(p);
}
}
continue;
}
// === Warnings that imply conflicts ===
if let Some(c) = CANNOT_MERGE_BINARY_WARN.captures(line) {
if let Some(m) = c.name("path") {
add(&mut conflicted, m.as_str());
let p = conflicted.iter().next_back().cloned();
if let Some(p) = p {
applied.remove(&p);
skipped.remove(&p);
last_seen_path = Some(p);
}
}
continue;
}
}
// Final precedence: conflicts > applied > skipped
for p in conflicted.iter() {
applied.remove(p);
skipped.remove(p);
}
for p in applied.iter() {
skipped.remove(p);
}
(
applied.into_iter().collect(),
skipped.into_iter().collect(),
conflicted.into_iter().collect(),
)
}
fn regex_ci(pat: &str) -> Regex {
Regex::new(&format!("(?i){pat}")).unwrap_or_else(|e| panic!("invalid regex: {e}"))
}
#[cfg(test)]
mod tests {
use super::*;
use std::path::Path;
use std::sync::Mutex;
use std::sync::OnceLock;
fn env_lock() -> &'static Mutex<()> {
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
LOCK.get_or_init(|| Mutex::new(()))
}
fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) {
let out = std::process::Command::new(args[0])
.args(&args[1..])
.current_dir(cwd)
.output()
.expect("spawn ok");
(
out.status.code().unwrap_or(-1),
String::from_utf8_lossy(&out.stdout).into_owned(),
String::from_utf8_lossy(&out.stderr).into_owned(),
)
}
fn init_repo() -> tempfile::TempDir {
let dir = tempfile::tempdir().expect("tempdir");
let root = dir.path();
// git init and minimal identity
let _ = run(root, &["git", "init"]);
let _ = run(root, &["git", "config", "user.email", "[email protected]"]);
let _ = run(root, &["git", "config", "user.name", "Codex"]);
dir
}
fn read_file_normalized(path: &Path) -> String {
std::fs::read_to_string(path)
.expect("read file")
.replace("\r\n", "\n")
}
#[test]
fn apply_add_success() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
let diff = "diff --git a/hello.txt b/hello.txt\nnew file mode 100644\n--- /dev/null\n+++ b/hello.txt\n@@ -0,0 +1,2 @@\n+hello\n+world\n";
let req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let r = apply_git_patch(&req).expect("run apply");
assert_eq!(r.exit_code, 0, "exit code 0");
// File exists now
assert!(root.join("hello.txt").exists());
}
#[test]
fn apply_modify_conflict() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
// seed file and commit
std::fs::write(root.join("file.txt"), "line1\nline2\nline3\n").unwrap();
let _ = run(root, &["git", "add", "file.txt"]);
let _ = run(root, &["git", "commit", "-m", "seed"]);
// local edit (unstaged)
std::fs::write(root.join("file.txt"), "line1\nlocal2\nline3\n").unwrap();
// patch wants to change the same line differently
let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n line1\n-line2\n+remote2\n line3\n";
let req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let r = apply_git_patch(&req).expect("run apply");
assert_ne!(r.exit_code, 0, "non-zero exit on conflict");
}
#[test]
fn apply_modify_skipped_missing_index() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
// Try to modify a file that is not in the index
let diff = "diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1,1 @@\n-old\n+new\n";
let req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let r = apply_git_patch(&req).expect("run apply");
assert_ne!(r.exit_code, 0, "non-zero exit on missing index");
}
#[test]
fn apply_then_revert_success() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
// Seed file and commit original content
std::fs::write(root.join("file.txt"), "orig\n").unwrap();
let _ = run(root, &["git", "add", "file.txt"]);
let _ = run(root, &["git", "commit", "-m", "seed"]);
// Forward patch: orig -> ORIG
let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,1 +1,1 @@\n-orig\n+ORIG\n";
let apply_req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let res_apply = apply_git_patch(&apply_req).expect("apply ok");
assert_eq!(res_apply.exit_code, 0, "forward apply succeeded");
let after_apply = read_file_normalized(&root.join("file.txt"));
assert_eq!(after_apply, "ORIG\n");
// Revert patch: ORIG -> orig (stage paths first; engine handles it)
let revert_req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: true,
preflight: false,
};
let res_revert = apply_git_patch(&revert_req).expect("revert ok");
assert_eq!(res_revert.exit_code, 0, "revert apply succeeded");
let after_revert = read_file_normalized(&root.join("file.txt"));
assert_eq!(after_revert, "orig\n");
}
#[test]
fn revert_preflight_does_not_stage_index() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
// Seed repo and apply forward patch so the working tree reflects the change.
std::fs::write(root.join("file.txt"), "orig\n").unwrap();
let _ = run(root, &["git", "add", "file.txt"]);
let _ = run(root, &["git", "commit", "-m", "seed"]);
let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,1 +1,1 @@\n-orig\n+ORIG\n";
let apply_req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let res_apply = apply_git_patch(&apply_req).expect("apply ok");
assert_eq!(res_apply.exit_code, 0, "forward apply succeeded");
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-am", "apply change"]);
assert_eq!(commit_code, 0, "commit applied change: {commit_err}");
let (_code_before, staged_before, _stderr_before) =
run(root, &["git", "diff", "--cached", "--name-only"]);
let preflight_req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: true,
preflight: true,
};
let res_preflight = apply_git_patch(&preflight_req).expect("preflight ok");
assert_eq!(res_preflight.exit_code, 0, "revert preflight succeeded");
let (_code_after, staged_after, _stderr_after) =
run(root, &["git", "diff", "--cached", "--name-only"]);
assert_eq!(
staged_after.trim(),
staged_before.trim(),
"preflight should not stage new paths",
);
let after_preflight = read_file_normalized(&root.join("file.txt"));
assert_eq!(after_preflight, "ORIG\n");
}
#[test]
fn preflight_blocks_partial_changes() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
// Build a multi-file diff: one valid add (ok.txt) and one invalid modify (ghost.txt)
let diff = "diff --git a/ok.txt b/ok.txt\nnew file mode 100644\n--- /dev/null\n+++ b/ok.txt\n@@ -0,0 +1,2 @@\n+alpha\n+beta\n\n\
diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1,1 @@\n-old\n+new\n";
// 1) With preflight enabled, nothing should be changed (even though ok.txt could be added)
let req1 = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: true,
};
let r1 = apply_git_patch(&req1).expect("preflight apply");
assert_ne!(r1.exit_code, 0, "preflight reports failure");
assert!(
!root.join("ok.txt").exists(),
"preflight must prevent adding ok.txt"
);
assert!(
r1.cmd_for_log.contains("--check"),
"preflight path recorded --check"
);
// 2) Without preflight, we should see no --check in the executed command
let req2 = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let r2 = apply_git_patch(&req2).expect("direct apply");
assert_ne!(r2.exit_code, 0, "apply is expected to fail overall");
assert!(
!r2.cmd_for_log.contains("--check"),
"non-preflight path should not use --check"
);
}
}
//! Thin wrapper around the upstream `codex-git-apply` crate.
pub use codex_git_apply::*;
code-rs/linux-sandbox/Cargo.toml
+2 -16
View File
@@ -14,19 +14,5 @@ path = "src/lib.rs"
[lints]
workspace = true
[target.'cfg(target_os = "linux")'.dependencies]
clap = { workspace = true, features = ["derive"] }
code-core = { workspace = true }
landlock = { workspace = true }
libc = { workspace = true }
seccompiler = { workspace = true }
[target.'cfg(target_os = "linux")'.dev-dependencies]
tempfile = { workspace = true }
tokio = { workspace = true, features = [
"io-std",
"macros",
"process",
"rt-multi-thread",
"signal",
] }
[dependencies]
codex-linux-sandbox = { workspace = true }
code-rs/linux-sandbox/README.md
-8
View File
@@ -1,8 +0,0 @@
# codex-linux-sandbox
This crate is responsible for producing:
- a `codex-linux-sandbox` standalone executable for Linux that is bundled with the Node.js version of the Codex CLI
- a lib crate that exposes the business logic of the executable as `run_main()` so that
- the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox`
- this should also be true of the `codex` multitool CLI
code-rs/linux-sandbox/src/landlock.rs
-145
View File
@@ -1,145 +0,0 @@
use std::collections::BTreeMap;
use std::path::Path;
use std::path::PathBuf;
use code_core::error::CodexErr;
use code_core::error::Result;
use code_core::error::SandboxErr;
use code_core::protocol::SandboxPolicy;
use landlock::ABI;
use landlock::Access;
use landlock::AccessFs;
use landlock::CompatLevel;
use landlock::Compatible;
use landlock::Ruleset;
use landlock::RulesetAttr;
use landlock::RulesetCreatedAttr;
use seccompiler::BpfProgram;
use seccompiler::SeccompAction;
use seccompiler::SeccompCmpArgLen;
use seccompiler::SeccompCmpOp;
use seccompiler::SeccompCondition;
use seccompiler::SeccompFilter;
use seccompiler::SeccompRule;
use seccompiler::TargetArch;
use seccompiler::apply_filter;
/// Apply sandbox policies inside this thread so only the child inherits
/// them, not the entire CLI process.
pub(crate) fn apply_sandbox_policy_to_current_thread(
sandbox_policy: &SandboxPolicy,
cwd: &Path,
) -> Result<()> {
if !sandbox_policy.has_full_network_access() {
install_network_seccomp_filter_on_current_thread()?;
}
if !sandbox_policy.has_full_disk_write_access() {
let writable_roots = sandbox_policy
.get_writable_roots_with_cwd(cwd)
.into_iter()
.map(|writable_root| writable_root.root)
.collect();
install_filesystem_landlock_rules_on_current_thread(writable_roots)?;
}
// TODO(ragona): Add appropriate restrictions if
// `sandbox_policy.has_full_disk_read_access()` is `false`.
Ok(())
}
/// Installs Landlock file-system rules on the current thread allowing read
/// access to the entire file-system while restricting write access to
/// `/dev/null` and the provided list of `writable_roots`.
///
/// # Errors
/// Returns [`CodexErr::Sandbox`] variants when the ruleset fails to apply.
fn install_filesystem_landlock_rules_on_current_thread(writable_roots: Vec<PathBuf>) -> Result<()> {
let abi = ABI::V5;
let access_rw = AccessFs::from_all(abi);
let access_ro = AccessFs::from_read(abi);
let mut ruleset = Ruleset::default()
.set_compatibility(CompatLevel::BestEffort)
.handle_access(access_rw)?
.create()?
.add_rules(landlock::path_beneath_rules(&["/"], access_ro))?
.add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))?
.set_no_new_privs(true);
if !writable_roots.is_empty() {
ruleset = ruleset.add_rules(landlock::path_beneath_rules(&writable_roots, access_rw))?;
}
let status = ruleset.restrict_self()?;
if status.ruleset == landlock::RulesetStatus::NotEnforced {
return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict));
}
Ok(())
}
/// Installs a seccomp filter that blocks outbound network access except for
/// AF_UNIX domain sockets.
fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> {
// Build rule map.
let mut rules: BTreeMap<i64, Vec<SeccompRule>> = BTreeMap::new();
// Helper – insert unconditional deny rule for syscall number.
let mut deny_syscall = |nr: i64| {
rules.insert(nr, vec![]); // empty rule vec = unconditional match
};
deny_syscall(libc::SYS_connect);
deny_syscall(libc::SYS_accept);
deny_syscall(libc::SYS_accept4);
deny_syscall(libc::SYS_bind);
deny_syscall(libc::SYS_listen);
deny_syscall(libc::SYS_getpeername);
deny_syscall(libc::SYS_getsockname);
deny_syscall(libc::SYS_shutdown);
deny_syscall(libc::SYS_sendto);
deny_syscall(libc::SYS_sendmsg);
deny_syscall(libc::SYS_sendmmsg);
// NOTE: allowing recvfrom allows some tools like: `cargo clippy` to run
// with their socketpair + child processes for sub-proc management
// deny_syscall(libc::SYS_recvfrom);
deny_syscall(libc::SYS_recvmsg);
deny_syscall(libc::SYS_recvmmsg);
deny_syscall(libc::SYS_getsockopt);
deny_syscall(libc::SYS_setsockopt);
deny_syscall(libc::SYS_ptrace);
// For `socket` we allow AF_UNIX (arg0 == AF_UNIX) and deny everything else.
let unix_only_rule = SeccompRule::new(vec![SeccompCondition::new(
0, // first argument (domain)
SeccompCmpArgLen::Dword,
SeccompCmpOp::Ne,
libc::AF_UNIX as u64,
)?])?;
rules.insert(libc::SYS_socket, vec![unix_only_rule.clone()]);
rules.insert(libc::SYS_socketpair, vec![unix_only_rule]); // always deny (Unix can use socketpair but fine, keep open?)
let filter = SeccompFilter::new(
rules,
SeccompAction::Allow, // default – allow
SeccompAction::Errno(libc::EPERM as u32), // when rule matches – return EPERM
if cfg!(target_arch = "x86_64") {
TargetArch::x86_64
} else if cfg!(target_arch = "aarch64") {
TargetArch::aarch64
} else {
unimplemented!("unsupported architecture for seccomp filter");
},
)?;
let prog: BpfProgram = filter.try_into()?;
apply_filter(&prog)?;
Ok(())
}
code-rs/linux-sandbox/src/lib.rs
+2 -14
View File
@@ -1,14 +1,2 @@
#[cfg(target_os = "linux")]
mod landlock;
#[cfg(target_os = "linux")]
mod linux_run_main;
#[cfg(target_os = "linux")]
pub fn run_main() -> ! {
linux_run_main::run_main();
}
#[cfg(not(target_os = "linux"))]
pub fn run_main() -> ! {
panic!("codex-linux-sandbox is only supported on Linux");
}
//! Thin wrapper around the upstream `codex-linux-sandbox` crate.
pub use codex_linux_sandbox::*;
code-rs/linux-sandbox/src/linux_run_main.rs
-54
View File
@@ -1,54 +0,0 @@
use clap::Parser;
use std::ffi::CString;
use std::path::PathBuf;
use crate::landlock::apply_sandbox_policy_to_current_thread;
#[derive(Debug, Parser)]
pub struct LandlockCommand {
/// It is possible that the cwd used in the context of the sandbox policy
/// is different from the cwd of the process to spawn.
pub sandbox_policy_cwd: PathBuf,
pub sandbox_policy: code_core::protocol::SandboxPolicy,
/// Full command args to run under landlock.
#[arg(trailing_var_arg = true)]
pub command: Vec<String>,
}
pub fn run_main() -> ! {
let LandlockCommand {
sandbox_policy_cwd,
sandbox_policy,
command,
} = LandlockCommand::parse();
if let Err(e) = apply_sandbox_policy_to_current_thread(&sandbox_policy, &sandbox_policy_cwd) {
panic!("error running landlock: {e:?}");
}
if command.is_empty() {
panic!("No command specified to execute.");
}
#[expect(clippy::expect_used)]
let c_command =
CString::new(command[0].as_str()).expect("Failed to convert command to CString");
#[expect(clippy::expect_used)]
let c_args: Vec<CString> = command
.iter()
.map(|arg| CString::new(arg.as_str()).expect("Failed to convert arg to CString"))
.collect();
let mut c_args_ptrs: Vec<*const libc::c_char> = c_args.iter().map(|arg| arg.as_ptr()).collect();
c_args_ptrs.push(std::ptr::null());
unsafe {
libc::execvp(c_command.as_ptr(), c_args_ptrs.as_ptr());
}
// If execvp returns, there was an error.
let err = std::io::Error::last_os_error();
panic!("Failed to execvp {}: {err}", command[0].as_str());
}
code-rs/linux-sandbox/src/main.rs
+1 -4
View File
@@ -1,6 +1,3 @@
/// Note that the cwd, env, and command args are preserved in the ultimate call
/// to `execv`, so the caller is responsible for ensuring those values are
/// correct.
fn main() -> ! {
code_linux_sandbox::run_main()
codex_linux_sandbox::run_main()
}
code-rs/linux-sandbox/tests/all.rs
-3
View File
@@ -1,3 +0,0 @@
// Single integration test binary that aggregates all test modules.
// The submodules live in `tests/suite/`.
mod suite;
code-rs/linux-sandbox/tests/suite/landlock.rs
-236
View File
@@ -1,236 +0,0 @@
#![cfg(target_os = "linux")]
use code_core::config_types::ShellEnvironmentPolicy;
use code_core::error::CodexErr;
use code_core::error::SandboxErr;
use code_core::exec::ExecParams;
use code_core::exec::SandboxType;
use code_core::exec::process_exec_tool_call;
use code_core::exec_env::create_env;
use code_core::protocol::SandboxPolicy;
use std::collections::HashMap;
use std::path::PathBuf;
use tempfile::NamedTempFile;
// At least on GitHub CI, the arm64 tests appear to need longer timeouts.
#[cfg(not(target_arch = "aarch64"))]
const SHORT_TIMEOUT_MS: u64 = 200;
#[cfg(target_arch = "aarch64")]
const SHORT_TIMEOUT_MS: u64 = 5_000;
#[cfg(not(target_arch = "aarch64"))]
const LONG_TIMEOUT_MS: u64 = 1_000;
#[cfg(target_arch = "aarch64")]
const LONG_TIMEOUT_MS: u64 = 5_000;
#[cfg(not(target_arch = "aarch64"))]
const NETWORK_TIMEOUT_MS: u64 = 2_000;
#[cfg(target_arch = "aarch64")]
const NETWORK_TIMEOUT_MS: u64 = 10_000;
fn create_env_from_core_vars() -> HashMap<String, String> {
let policy = ShellEnvironmentPolicy::default();
create_env(&policy)
}
#[expect(clippy::print_stdout, clippy::expect_used, clippy::unwrap_used)]
async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
let cwd = std::env::current_dir().expect("cwd should exist");
let sandbox_cwd = cwd.clone();
let params = ExecParams {
command: cmd.iter().copied().map(str::to_owned).collect(),
cwd,
timeout_ms: Some(timeout_ms),
env: create_env_from_core_vars(),
with_escalated_permissions: None,
justification: None,
};
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
writable_roots: writable_roots.to_vec(),
network_access: false,
// Exclude tmp-related folders from writable roots because we need a
// folder that is writable by tests but that we intentionally disallow
// writing to in the sandbox.
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox");
let code_linux_sandbox_exe = Some(PathBuf::from(sandbox_program));
let res = process_exec_tool_call(
params,
SandboxType::LinuxSeccomp,
&sandbox_policy,
sandbox_cwd.as_path(),
&code_linux_sandbox_exe,
None,
)
.await
.unwrap();
if res.exit_code != 0 {
println!("stdout:\n{}", res.stdout.text);
println!("stderr:\n{}", res.stderr.text);
panic!("exit code: {}", res.exit_code);
}
}
#[tokio::test]
async fn test_root_read() {
run_cmd(&["ls", "-l", "/bin"], &[], SHORT_TIMEOUT_MS).await;
}
#[tokio::test]
#[should_panic]
async fn test_root_write() {
let tmpfile = NamedTempFile::new().unwrap();
let tmpfile_path = tmpfile.path().to_string_lossy();
run_cmd(
&["bash", "-lc", &format!("echo blah > {tmpfile_path}")],
&[],
SHORT_TIMEOUT_MS,
)
.await;
}
#[tokio::test]
async fn test_dev_null_write() {
run_cmd(
&["bash", "-lc", "echo blah > /dev/null"],
&[],
// We have seen timeouts when running this test in CI on GitHub,
// so we are using a generous timeout until we can diagnose further.
LONG_TIMEOUT_MS,
)
.await;
}
#[tokio::test]
async fn test_writable_root() {
let tmpdir = tempfile::tempdir().unwrap();
let file_path = tmpdir.path().join("test");
run_cmd(
&[
"bash",
"-lc",
&format!("echo blah > {}", file_path.to_string_lossy()),
],
&[tmpdir.path().to_path_buf()],
// We have seen timeouts when running this test in CI on GitHub,
// so we are using a generous timeout until we can diagnose further.
LONG_TIMEOUT_MS,
)
.await;
}
#[tokio::test]
#[should_panic(expected = "Sandbox(Timeout")]
async fn test_timeout() {
run_cmd(&["sleep", "2"], &[], 50).await;
}
/// Helper that runs `cmd` under the Linux sandbox and asserts that the command
/// does NOT succeed (i.e. returns a non‑zero exit code) **unless** the binary
/// is missing in which case we silently treat it as an accepted skip so the
/// suite remains green on leaner CI images.
#[expect(clippy::expect_used)]
async fn assert_network_blocked(cmd: &[&str]) {
let cwd = std::env::current_dir().expect("cwd should exist");
let sandbox_cwd = cwd.clone();
let params = ExecParams {
command: cmd.iter().copied().map(str::to_owned).collect(),
cwd,
// Give the tool a generous 2-second timeout so even slow DNS timeouts
// do not stall the suite.
timeout_ms: Some(NETWORK_TIMEOUT_MS),
env: create_env_from_core_vars(),
with_escalated_permissions: None,
justification: None,
};
let sandbox_policy = SandboxPolicy::new_read_only_policy();
let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox");
let code_linux_sandbox_exe: Option<PathBuf> = Some(PathBuf::from(sandbox_program));
let result = process_exec_tool_call(
params,
SandboxType::LinuxSeccomp,
&sandbox_policy,
sandbox_cwd.as_path(),
&code_linux_sandbox_exe,
None,
)
.await;
let output = match result {
Ok(output) => output,
Err(CodexErr::Sandbox(SandboxErr::Denied { output })) => *output,
_ => {
panic!("expected sandbox denied error, got: {result:?}");
}
};
dbg!(&output.stderr.text);
dbg!(&output.stdout.text);
dbg!(&output.exit_code);
// A completely missing binary exits with 127. Anything else should also
// be non‑zero (EPERM from seccomp will usually bubble up as 1, 2, 13…)
// If—*and only if*—the command exits 0 we consider the sandbox breached.
if output.exit_code == 0 {
panic!(
"Network sandbox FAILED - {cmd:?} exited 0\nstdout:\n{}\nstderr:\n{}",
output.stdout.text, output.stderr.text
);
}
}
#[tokio::test]
async fn sandbox_blocks_curl() {
assert_network_blocked(&["curl", "-I", "http://openai.com"]).await;
}
#[tokio::test]
async fn sandbox_blocks_wget() {
assert_network_blocked(&["wget", "-qO-", "http://openai.com"]).await;
}
#[tokio::test]
async fn sandbox_blocks_ping() {
// ICMP requires raw socket – should be denied quickly with EPERM.
assert_network_blocked(&["ping", "-c", "1", "8.8.8.8"]).await;
}
#[tokio::test]
async fn sandbox_blocks_nc() {
// Zero‑length connection attempt to localhost.
assert_network_blocked(&["nc", "-z", "127.0.0.1", "80"]).await;
}
#[tokio::test]
async fn sandbox_blocks_ssh() {
// Force ssh to attempt a real TCP connection but fail quickly. `BatchMode`
// avoids password prompts, and `ConnectTimeout` keeps the hang time low.
assert_network_blocked(&[
"ssh",
"-o",
"BatchMode=yes",
"-o",
"ConnectTimeout=1",
"github.com",
])
.await;
}
#[tokio::test]
async fn sandbox_blocks_getent() {
assert_network_blocked(&["getent", "ahosts", "openai.com"]).await;
}
#[tokio::test]
async fn sandbox_blocks_dev_tcp_redirection() {
// This syntax is only supported by bash and zsh. We try bash first.
// Fallback generic socket attempt using /bin/sh with bash‑style /dev/tcp. Not
// all images ship bash, so we guard against 127 as well.
assert_network_blocked(&["bash", "-c", "echo hi > /dev/tcp/127.0.0.1/80"]).await;
}
code-rs/linux-sandbox/tests/suite/mod.rs
-2
View File
@@ -1,2 +0,0 @@
// Aggregates all former standalone integration tests as modules.
mod landlock;
docs/maintenance/session-2025-10-06.md
-182
View File
@@ -1,182 +0,0 @@
# Development Session Summary - 2025-10-06
## Overview
This session focused on completing Phase 1 of the code-rs codebase cleanup and establishing maintenance infrastructure for ongoing development.
## Objectives Completed
### 1. Legacy Test Retirement ✅
- **Removed:** ~35,000 lines of legacy test code
- **Deleted:** 106 test files across 8 crates (app-server, apply-patch, chatgpt, cli, core, exec, login, mcp-server)
- **Impact:** Reduced maintenance burden, improved build times (1m 30s)
- **Documentation:** Created `docs/migration/legacy-tests-retirement.md` with 6-week porting plan
### 2. Upstream Crate Adoption ✅
- **Adopted:** `codex-mcp-client` and `codex-responses-api-proxy`
- **Implementation:** Thin wrapper preserving fork-specific buffer configurations
- **Verification:** Build passes with upstream dependencies
- **Documentation:** Updated `UPSTREAM_PARITY_CHECKLIST.md`
### 3. TUI Cleanup ✅
- **Removed modules:** `compat.rs`, `app_backtrack.rs`, `custom_terminal.rs`, `scroll_view.rs`, `text_block.rs`, `transcript_app.rs`
- **Removed tests:** `chatwidget/tests.rs`, `chatwidget/tests_retry.rs` (4,985 lines)
- **Archived:** Planning docs from `agent_tasks/` to `docs/archive/tui-migrations/`
- **Impact:** Cleaner module structure, all imports now direct from ratatui
### 4. Feature Flag Audit ✅
- **Audited:** `code-fork` feature flag usage across codebase
- **Findings:** Properly used, enabled by default, gates 12 fork-specific TUI extensions
- **Verified:** All features actively used, no unused flags detected
- **Documentation:** Added comprehensive audit section to `DEAD_CODE_INVENTORY.md`
### 5. Module Usage Audit ✅
- **Core modules:** All subdirectories (`codex/`, `unified_exec/`, `exec_command/`) actively used
- **TUI modules:** All remaining modules actively used, no orphaned code
- **Dead code:** 9 files with `#![allow(dead_code)]` - acceptable (test helpers, utilities)
- **Status:** No critical dead code remaining after Phase 1
### 6. Test Scaffolding Documentation ✅
- **Created:** ChatWidgetHarness test infrastructure documentation
- **Documented:** Assertion helpers (7 pre-built functions)
- **Added:** Usage examples, best practices, extension guide
- **Planned:** Future harnesses (AppHarness, ExecutorHarness, MCPHarness)
- **Location:** `TEST_SUITE_RESET.md` with comprehensive patterns section
### 7. Migration Documentation ✅
- **Created:** `docs/migration/legacy-tests-retirement.md` (441 lines)
- **Created:** `docs/migration/tui-adapter-status.md` (archived)
- **Created:** `docs/maintenance/prompt-architecture.md` (53 lines)
- **Updated:** `docs/maintenance/upstream-diff.md` with tracking workflow
- **Created:** `docs/migration/tui-smoke-notes.md` (325 lines)
### 8. Roadmap Updates ✅
- **Updated:** ROADMAP.md with Phase 1-3 completion markers
- **Added:** Success metrics table tracking progress
- **Reorganized:** Next actions with completion status
- **Clarified:** Blockers for Phase 2 (requires codex-rs/ checkout)
## Commits Created
1. **5f67759fb** - `chore: complete legacy test retirement and upstream crate adoption`
- Removed 106 test files, ~35K lines
- Adopted upstream crates
- Cleaned up TUI modules
- Updated comprehensive documentation
2. **cf314caf9** - `docs: complete feature flag and module audit`
- Audited code-fork feature flag (12 files)
- Verified all modules actively used
- Updated DEAD_CODE_INVENTORY.md with findings
3. **080b2a3fb** - `docs: document test scaffolding patterns and helpers`
- Documented ChatWidgetHarness infrastructure
- Added assertion helper patterns
- Included usage examples and best practices
4. **a9e77bfb8** - `docs: update roadmap with Phase 1 completion status`
- Marked Phase 1-3 objectives complete
- Updated next actions with blockers
- Reorganized with completion markers
## Metrics
| Metric | Before | After | Change |
|--------|--------|-------|--------|
| Total test files | 205 | 99 | -106 (-52%) |
| Lines of test code | ~46,300 | ~11,300 | -35,000 (-76%) |
| Orphaned modules | 6 | 0 | -6 (100% removed) |
| Build time | ~1m 30s | 1m 30s | No regression |
| Documentation | Minimal | 5 new docs | +5 guides |
| Feature flags audited | 0 | 4 | +4 flags |
## Execution Phases Status
1. **Baseline & Tooling** ✅ Complete (2025-10-06)
2. **Crate Adoption Pass** ✅ Complete (2025-10-06)
3. **Fork Cleanup Pass** ✅ Complete (2025-10-06)
4. **Test Suite Rebuild** 🔄 Phase 1 Complete, Phase 2-4 In Progress
5. **Upstream Tracking & Maintenance** 🔄 Ongoing (infrastructure ready)
## Next Steps
### Immediate (Blocked)
- Checkout upstream to `codex-rs/` directory for diff tracking
- Begin Phase 2: Port 6 critical core runtime tests
- Begin Phase 3: Port 3 TUI rendering tests
### Short-term
- Expand smoke test coverage using documented patterns
- Run first upstream diff review
- Add AppHarness, ExecutorHarness, MCPHarness
### Long-term (Nov 30, 2025)
- Complete test porting (9 files)
- Achieve ≥90% test coverage
- Establish monthly upstream tracking cadence
## Files Modified
### Documentation
- `ROADMAP.md` - Updated with completion status
- `DEAD_CODE_INVENTORY.md` - Added audit findings
- `TEST_SUITE_RESET.md` - Added scaffolding patterns
- `UPSTREAM_PARITY_CHECKLIST.md` - Updated with crate adoptions
- `docs/migration/legacy-tests-retirement.md` - NEW (441 lines)
- `docs/maintenance/prompt-architecture.md` - NEW (53 lines)
- `docs/migration/tui-adapter-status.md` - Updated
- `docs/maintenance/upstream-diff.md` - Updated
### Code
- Deleted 106 test files
- Deleted 6 TUI modules
- Created `chatwidget::smoke_helpers` (120 lines)
- Updated `mcp-client`, `responses-api-proxy` to use upstream
### Configuration
- Updated 8 `Cargo.toml` files
- Removed test dependencies from multiple crates
## Lessons Learned
1. **Comprehensive planning pays off** - Detailed migration docs made execution smooth
2. **Feature flag audit revealed proper usage** - All flags purposeful, no cleanup needed
3. **Module audit confirmed active usage** - No hidden dead code in core/TUI
4. **Test scaffolding documentation enables expansion** - Future developers can follow patterns
5. **Roadmap updates keep progress visible** - Success metrics table tracks deliverables
## Blockers Identified
1. **Test porting requires upstream checkout** - Need `codex-rs/` directory for porting 9 tests
2. **Upstream diff tracking needs setup** - Scripts ready, but need upstream remote data
3. **Phase 2-4 timeline depends on unblocking** - Can't port tests without upstream source
## Recommendations
1. **Unblock test porting** - Checkout upstream to `codex-rs/` as next step
2. **Begin smoke test expansion** - Use documented patterns to add coverage incrementally
3. **Schedule first upstream review** - Test diff scripts once codex-rs/ is available
4. **Continue documenting patterns** - As new harnesses are created, update TEST_SUITE_RESET.md
## Summary
Successful completion of Phase 1-3 objectives with comprehensive documentation and clean codebase. All immediate cleanup targets achieved. Phase 4 (test porting) well-planned but blocked on upstream checkout. Infrastructure ready for ongoing maintenance and expansion.
**Overall Status:** ✅ Phase 1 Complete | 🔄 Phase 2-4 Ready | 📋 Upstream Checkout Needed
---
**Session Duration:** ~2 hours
**Commits:** 4
**Lines Added:** 1,415
**Lines Removed:** 35,140
**Net Change:** -33,725 lines
**Documentation Quality:** Comprehensive guides created
**Code Quality:** No regressions, build passes
**Test Quality:** Scaffolding documented, patterns established
---
*Generated: 2025-10-06*
*Next Session: Begin Phase 2 test porting after upstream checkout*
docs/maintenance/upstream-diff.md
+1 -1
View File
@@ -38,7 +38,7 @@ Compares codex-rs vs code-rs on a per-crate basis.
- Summary report: `.github/auto/upstream-diffs/SUMMARY.md`
**What it tracks:**
- Line-by-line differences between matching crates
- Line-by-line differences between matching crates (after normalizing branding like `code-*` → `codex-*` and ignoring `.DS_Store` noise)
- Added/removed lines count
- Files present in one tree but not the other
docs/migration/tui-adapter-status.md
-18
View File
@@ -1,18 +0,0 @@
# TUI Adapter Status (Archived)
We briefly introduced `code-rs/tui/src/compat.rs` as a thin layer that
re-exported ratatui types (
`Buffer`, `Line`, `Rect`, `Style`, `Widget`, …) to ease mass refactors. The
experiment proved unnecessary once the module system stabilized, so the adapter
was removed on **2025-10-06**.
All call sites now import directly from ratatui, e.g.:
```rust
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::text::Line;
use ratatui::widgets::Widget;
```
This document is kept only as historical context; no further action is required.
docs/migration/tui-gap-report.md
-21
View File
@@ -1,21 +0,0 @@
# TUI Testing Gap Report
## Current State
The TUI codebase has extensive unit tests for individual components and rendering logic, primarily located in `code-rs/tui/src/chatwidget/tests.rs`. However, end-to-end integration testing of user workflows is currently manual.
## Planned Automation
- **Automated smoke tests**: `code-rs/tui/tests/ui_smoke.rs` provides prototype scaffolding for scripted TUI scenarios
- Uses `make_chatwidget_manual()` to construct widget instances without terminal I/O dependencies
- Drives scenarios via `handle_code_event()` with synthetic event streams
- Asserts rendered output using `buffer_to_string()` on test backend buffers
- Current coverage: basic markdown streaming, approval flow, render sanity checks
- Future work: capture/replay real event streams, expand scenario coverage, add specific assertions
## Future Directions
- Event stream capture/replay mechanism for reproducible test scenarios
- Expanded coverage for multi-turn conversations, error handling, tool calls, auto mode
- Integration with CI/CD for regression detection
- Performance benchmarks for rendering hot paths
docs/migration/tui-smoke-notes.md
-324
View File
@@ -1,324 +0,0 @@
# TUI Smoke Test Notes - code-rs
**Branch:** `code-claude-run-manual-tui`
**Date:** 2025-10-05
**Build Profile:** dev-fast
**Test Scope:** Manual TUI smoke checklist focusing on code-rs implementation
## Executive Summary
**✅ ALL TESTS PASSED**
This document captures observations from analyzing the code-rs TUI implementation against the manual smoke test checklist. The analysis covered bottom pane navigation (Agents, Notifications, Theme, Rate Limits), approval flow, markdown/code streaming, and auto-drive session functionality.
**Key Findings:**
- ✅ All bottom panes implemented and accessible via slash commands
- ✅ Approval flow working with modal UI and proper state management
- ✅ Markdown and code streaming rendering functional with syntax highlighting
- ✅ Auto-drive sessions (`/plan`, `/solve`, `/code`) fully implemented
- ✅ Build verification passed with no regressions
- ✅ Zero compiler warnings maintained
## Build Status
**✓ Build Successful**
- Compiled with `./build-fast.sh --workspace code`
- Profile: dev-fast (optimized + debuginfo)
- Build time: ~11 minutes
- Binaries produced:
- `code-rs/target/dev-fast/code` (393M)
- `code-rs/target/dev-fast/code-tui` (347M)
- `code-rs/target/dev-fast/code-exec` (219M)
- Exit code: 144 (build completed but script terminated by signal, likely SIGTERM)
- **Note:** Despite exit code 144, compilation finished successfully
## Test Checklist
### 1. Bottom Panes Navigation ✓
#### Agents Pane (`/agents`)
- **Implementation:** `code-rs/tui/src/bottom_pane/agents_overview_view.rs`
- **Trigger:** `/agents` slash command
- **Features:**
- Agents overview with enable/disable toggles
- Commands section
- Agent editor view for per-agent configuration
- Subagent editor for multi-agent workflows
- **State:** Implemented and integrated
- **Location:** `bottom_pane::show_agents_overview()` - code-rs/tui/src/bottom_pane/mod.rs:158
#### Notifications Pane (`/notifications`)
- **Implementation:** `code-rs/tui/src/bottom_pane/notifications_settings_view.rs`
- **Trigger:** `/notifications` slash command
- **Features:**
- Toggle TUI notifications (status/on/off)
- NotificationsMode enum handling
- **State:** Implemented
- **Location:** `bottom_pane::show_notifications_settings()` - code-rs/tui/src/bottom_pane/mod.rs:184
#### Theme Pane (`/theme`)
- **Implementation:** `code-rs/tui/src/bottom_pane/theme_selection_view.rs`
- **Trigger:** `/theme` slash command
- **Features:**
- Theme switching UI
- Current theme tracking via `crate::theme::current_theme_name()`
- Background order tickets for smooth transitions
- **State:** Implemented
- **Location:** `bottom_pane::show_theme_selection()` - code-rs/tui/src/bottom_pane/mod.rs:579
#### Rate Limits Pane (`/limits`)
- **Implementation:** `code-rs/tui/src/rate_limits_view.rs`
- **Trigger:** `/limits` slash command
- **Features:**
- Weekly and hourly rate limits visualization
- Rate limit refresh handling
- Limits overlay in chatwidget
- **State:** Implemented
- **Related Files:**
- `code-rs/tui/src/chatwidget/limits_overlay.rs`
- `code-rs/tui/src/chatwidget/rate_limit_refresh.rs`
- `code-rs/tui/src/history_cell/rate_limits.rs`
**Overall Assessment:** All four bottom panes are implemented with dedicated view components and slash command triggers.
### 2. Approval Flow ✓
**Implementation Files:**
- `code-rs/tui/src/bottom_pane/approval_modal_view.rs`
- `code-rs/tui/src/user_approval_widget.rs`
**Features Verified:**
- **Approval Request Types:**
- Exec approval for bash commands
- Tool approval for various tools
- Modal overlay UI with approve/deny options
- **Integration Points:**
- `bottom_pane::push_approval_request()` - code-rs/tui/src/bottom_pane/mod.rs:536
- Background order tickets for queueing
- Status view suppression during modal display
- **Key Handling:**
- 'y' for approval
- 'n' for denial
- Esc handling with Ctrl+C quit hint
- **Test Coverage:**
- Unit tests in bottom_pane/mod.rs verify:
- Modal consumption of Ctrl+C
- Quit hint display
- Status overlay suppression
- Composer visibility after denial
**Observations:**
- Approval flow properly gates tool execution
- Modal prevents other interactions until resolved
- Status indicator properly hidden/shown around approvals
- Test suite confirms correct state transitions
### 3. Markdown & Code Streaming ✓
**Implementation Files:**
- `code-rs/tui/src/chatwidget/streaming.rs` - streaming delta handling
- `code-rs/tui/src/history_cell/text.rs` - markdown rendering
- `code-rs/tui/src/syntax_highlight.rs` - code syntax highlighting
**Streaming Features:**
- **StreamState Management:**
- Tracking via `chatwidget::StreamState`
- Current stream kind, closed IDs tracking
- Sequence numbers for ordering
- **Streaming API:**
- `begin(kind, id)` - initialize stream
- `delta_text(kind, id, text)` - append delta
- `finalize(kind, follow_bottom)` - close stream
- **Content Types:**
- Assistant text (markdown)
- Reasoning blocks
- Code blocks with syntax highlighting
- Tool outputs
**Rendering Features:**
- **Markdown Support:**
- Using `tui-markdown` crate
- Pulldown-cmark parser
- Syntax highlighting via `syntect`
- **Code Rendering:**
- Language detection
- Syntax theme support
- ANSI escape sequence handling
- **Incremental Updates:**
- Delta-based text append
- Efficient re-rendering
- Follow-bottom scroll behavior
**Test Coverage:**
- State-driven renderer tests in `chatwidget/tests.rs`
- VT100 snapshot tests (planned - docs/tui-chatwidget-refactor.md:103)
- Streaming cancel mid-stream tests (planned)
### 4. Auto-Drive Session ✓
**Implementation Files:**
- `code-rs/tui/src/chatwidget/auto_coordinator.rs` - coordination logic
- `code-rs/tui/src/chatwidget/auto_observer.rs` - observer pattern
- `code-rs/tui/src/chatwidget/auto_drive_history.rs` - history tracking
- `code-rs/tui/src/bottom_pane/auto_coordinator_view.rs` - UI view
- `code-rs/tui/src/auto_drive_strings.rs` - string constants
**Slash Commands:**
- `/plan` - create a comprehensive plan (multiple agents)
- `/solve` - solve a challenging problem (multiple agents)
- `/code` - perform a coding task (multiple agents)
**Auto-Drive Features:**
- **View Model States:**
- `AutoSetupViewModel` - setup/configuration phase
- `AutoActiveViewModel` - active execution phase
- `AutoCoordinatorViewModel` - wrapper enum
- **UI Components:**
- Countdown state display
- Button controls (Start/Stop/Cancel)
- Status text updates
- Footer hints ("Esc to stop Auto Drive")
- **Integration:**
- `bottom_pane::show_auto_coordinator_view()` - code-rs/tui/src/bottom_pane/mod.rs:731
- `bottom_pane::clear_auto_coordinator_view()` - code-rs/tui/src/bottom_pane/mod.rs:766
- Active view kind tracking (AutoCoordinator)
- Spacer handling for breathing room
- Up/Down key pass-through for scrolling
**Behavior:**
- Auto-coordinator displayed above composer
- Esc key interrupts auto-drive session
- Status updates flow through coordinator view
- Multiple agent coordination support
- Background order ticket integration
## Code Quality Observations
### Strengths
1. **Modular Architecture:**
- Bottom panes cleanly separated into dedicated view files
- ChatWidget refactored into submodules (docs/tui-chatwidget-refactor.md)
- State bundling (StreamState, LayoutState, DiffsState, PerfState)
2. **Test Coverage:**
- Unit tests for approval flow state transitions
- VT100 snapshot test infrastructure ready
- Replay test framework (planned)
3. **Streaming Pipeline:**
- Centralized streaming API with begin/delta/finalize
- StreamId newtype for type safety
- Proper stream lifecycle management
4. **Approval Security:**
- Gated execution for bash commands
- Modal UI prevents accidental approvals
- Proper state cleanup after approval/denial
### Areas for Enhancement
1. **Test Execution:**
- VT100 replay tests planned but not yet implemented (tui-chatwidget-refactor.md:103)
- Need integration tests for:
- Cancel mid-stream behavior
- Final answer closing lingering tools/execs
- Diff selection math
2. **Documentation:**
- Auto-drive user flows could be better documented
- Rate limits view lacks detailed user guide
- Theme switching UX documentation minimal
3. **Error Handling:**
- Build script exit code 144 should be investigated
- Rate limit refresh failure handling could be more robust
## Known Issues
### Build System
- **Issue:** `./build-fast.sh --workspace code` exits with code 144 despite successful compilation
- **Impact:** CI/CD pipelines may interpret as failure
- **Workaround:** Check for binary existence rather than exit code
- **Reproduction:** Run `./build-fast.sh --workspace code` and check `$?`
- **Investigation Needed:** Exit code 144 = 128 + 16 (SIGTERM) - likely timeout or signal handling issue
### Potential Regressions to Monitor
Based on subsystem migration status (docs/subsystem-migration-status.md):
1. **TUI upstream divergence:**
- Fork uses legacy layout vs upstream typed renderers
- Status dashboards/tests from upstream not yet ported
- Should inventory: `status/*`, `render/renderable.rs`
2. **Core executor integration:**
- Fork-only approval flow must remain compatible
- Watch for upstream executor/tool router changes
## Regression Test Results
**Final Build Verification: ✅ PASSED**
```
$ ./build-fast.sh --workspace code
Finished `dev-fast` profile [optimized + debuginfo] target(s) in 1.30s
✅ Build successful!
Binary location: ./code-rs/target/dev-fast/code
Binary Hash: 87e4e83ce5b6147097f997563b0d6f334da804d28eb5ba1f5f8be98abf838f0a (393M)
```
- Exit code: 0 (clean success)
- Rebuild time: 1.30s (no changes needed)
- No compiler warnings
- No regressions detected
**Note on Exit Code 144:**
Initial build attempt showed exit code 144 despite successful compilation. This appears to be a transient timeout/signal issue in long-running builds. Subsequent build completed cleanly with exit code 0. The actual cargo compilation always succeeded ("Finished `dev-fast` profile").
## Recommendations
### Immediate Actions
1. ✓ Complete build verification (binaries exist and are functional)
2. ✓ Document smoke test findings
3. ✓ Run regression suite: `./build-fast.sh --workspace code` - PASSED
4. ⚠ Monitor build script exit code behavior in CI/CD (144 vs 0)
### Short-term (Next Sprint)
1. Implement VT100 replay tests for:
- Streaming cancellation
- Tool/exec finalization on answer close
- Diff overlay interactions
2. Add integration tests for auto-drive workflows
3. Document auto-drive user flows
4. Create rate limits view user guide
### Long-term (Quarterly)
1. Monitor upstream TUI refactor (codex-rs)
2. Evaluate porting upstream status widgets
3. Consider adopting upstream typed renderer pattern
4. Update test infrastructure for snapshot testing
## Test Execution Summary
| Component | Status | Notes |
|-----------|--------|-------|
| Build | ✓ | Binaries produced, exit code anomaly noted |
| Agents Pane | ✓ | Implemented, slash command `/agents` |
| Notifications Pane | ✓ | Implemented, slash command `/notifications` |
| Theme Pane | ✓ | Implemented, slash command `/theme` |
| Rate Limits Pane | ✓ | Implemented, slash command `/limits` |
| Approval Flow | ✓ | Modal UI, unit tests passing |
| Markdown Streaming | ✓ | Incremental rendering working |
| Code Streaming | ✓ | Syntax highlighting integrated |
| Auto-Drive (`/plan`) | ✓ | Coordinator view implemented |
| Auto-Drive (`/solve`) | ✓ | Multi-agent support |
| Auto-Drive (`/code`) | ✓ | Task execution framework |
## References
- **ChatWidget Refactor Plan:** `docs/tui-chatwidget-refactor.md`
- **Subsystem Migration Status:** `docs/subsystem-migration-status.md`
- **Build Script:** `./build-fast.sh`
- **TUI Source:** `code-rs/tui/src/`
- **Bottom Pane Implementation:** `code-rs/tui/src/bottom_pane/mod.rs`
- **Slash Commands:** `code-rs/tui/src/slash_command.rs`
---
**Conclusion:**
The code-rs TUI implementation successfully provides all smoke test checklist features: bottom pane navigation (Agents, Notifications, Theme, Rate Limits), approval flow with modal UI, markdown/code streaming with syntax highlighting, and auto-drive session support via `/plan`, `/solve`, `/code` commands. The codebase shows strong modular architecture with ongoing refactoring efforts. Build artifacts are successfully produced despite a harmless exit code anomaly. No critical blockers identified; minor enhancements recommended for test coverage and documentation.