diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..31205d7 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,22 @@ +# Ensure shell scripts always use LF line endings on all platforms +*.sh text eol=lf + +# Ensure Python files use LF line endings +*.py text eol=lf + +# Auto-detect text files and normalize line endings to LF +* text=auto eol=lf + +# Explicitly declare files that should always be treated as binary +*.exe binary +*.png binary +*.jpg binary +*.jpeg binary +*.gif binary +*.ico binary +*.mov binary +*.mp4 binary +*.mp3 binary +*.zip binary +*.gz binary +*.tar binary diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0f543d6..46c86a8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -104,38 +104,10 @@ jobs: cp install.sh install.bat release/ ls -la release/ - - name: Extract release notes from CHANGELOG - id: extract_notes - run: | - VERSION=${GITHUB_REF#refs/tags/v} - - # Extract version section from CHANGELOG.md - awk -v ver="$VERSION" ' - /^## [0-9]+\.[0-9]+\.[0-9]+ - / { - if (found) exit - if ($2 == ver) { - found = 1 - next - } - } - found && /^## / { exit } - found { print } - ' CHANGELOG.md > release_notes.md - - # Fallback to auto-generated if extraction failed - if [ ! -s release_notes.md ]; then - echo "⚠️ No release notes found in CHANGELOG.md for version $VERSION" > release_notes.md - echo "" >> release_notes.md - echo "## What's Changed" >> release_notes.md - echo "See commits in this release for details." >> release_notes.md - fi - - cat release_notes.md - - name: Create Release uses: softprops/action-gh-release@v2 with: files: release/* - body_path: release_notes.md + generate_release_notes: true draft: false prerelease: false diff --git a/.gitignore b/.gitignore index 37eed6d..f7448c5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,7 @@ .claude/ .claude-trace +.DS_Store +**/.DS_Store .venv .pytest_cache __pycache__ diff --git a/CHANGELOG.md b/CHANGELOG.md index b9cbf86..68f45b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,129 +1,712 @@ # Changelog -## 5.2.0 - 2025-12-13 +All notable changes to this project will be documented in this file. -### 🚀 Core Features +## [5.2.4] - 2025-12-16 -#### Skills System Enhancements -- **New Skills**: Added `codeagent`, `product-requirements`, `prototype-prompt-generator` to `skill-rules.json` -- **Auto-Activation**: Skills automatically trigger based on keyword/pattern matching via hooks -- **Backward Compatibility**: Retained `skills/codex/SKILL.md` for existing workflows -#### Multi-Backend Support (codeagent-wrapper) -- **Renamed**: `codex-wrapper` → `codeagent-wrapper` with pluggable backend architecture -- **Three Backends**: Codex (default), Claude, Gemini via `--backend` flag -- **Smart Parser**: Auto-detects backend JSON stream formats -- **Session Resume**: All backends support `-r ` cross-session resume -- **Parallel Execution**: DAG task scheduling with global and per-task backend configuration -- **Concurrency Control**: `CODEAGENT_MAX_PARALLEL_WORKERS` env var limits concurrent tasks (max 100) -- **Test Coverage**: 93.4% (backend.go 100%, config.go 97.8%, executor.go 96.4%) +### ⚙️ Miscellaneous Tasks -#### Dev Workflow -- **`/dev`**: 6-step minimal dev workflow with mandatory 90% test coverage -#### Hooks System -- **UserPromptSubmit**: Auto-activate skills based on context -- **PostToolUse**: Auto-validation/formatting after tool execution -- **Stop**: Cleanup and reporting on session end -- **Examples**: Skill auto-activation, pre-commit checks +- integrate git-cliff for automated changelog generation -#### Skills System -- **Auto-Activation**: `skill-rules.json` regex trigger rules -- **codeagent skill**: Multi-backend wrapper integration -- **Modular Design**: Easy to extend with custom skills +- bump version to 5.2.4 -#### Installation System Enhancements -- **`merge_json` operation**: Auto-merge `settings.json` configuration -- **Modular Installation**: `python3 install.py --module dev` -- **Verbose Logging**: `--verbose/-v` enables terminal real-time output -- **Streaming Output**: `op_run_command` streams bash script execution -- **Configuration Cleanup**: Removed deprecated `gh` module from `config.json` +### 🐛 Bug Fixes + + +- 防止 Claude backend 无限递归调用 + +- isolate log files per task in parallel mode + +### 💼 Other + + +- Merge pull request #70 from cexll/fix/prevent-codeagent-infinite-recursion + +- Merge pull request #69 from cexll/myclaude-master-20251215-073053-338465000 + +- update CHANGELOG.md + +- Merge pull request #65 from cexll/fix/issue-64-buffer-overflow + +## [5.2.3] - 2025-12-15 + + +### 🐛 Bug Fixes + + +- 修复 bufio.Scanner token too long 错误 ([#64](https://github.com/cexll/myclaude/issues/64)) + +### 💼 Other + + +- change version + +### 🧪 Testing + + +- 同步测试中的版本号至 5.2.3 + +## [5.2.2] - 2025-12-13 + + +### ⚙️ Miscellaneous Tasks + + +- Bump version and clean up documentation + +### 🐛 Bug Fixes + + +- fix codeagent backend claude no auto + +- fix install.py dev fail + +### 🧪 Testing + + +- Fix tests for ClaudeBackend default --dangerously-skip-permissions + +## [5.2.1] - 2025-12-13 + + +### 🐛 Bug Fixes + + +- fix codeagent claude and gemini root dir + +### 💼 Other + + +- update readme + +## [5.2.0] - 2025-12-13 + + +### ⚙️ Miscellaneous Tasks + + +- Update CHANGELOG and remove deprecated test files + +### 🐛 Bug Fixes + + +- fix race condition in stdout parsing + +- add worker limit cap and remove legacy alias + +- use -r flag for gemini backend resume + +- clarify module list shows default state not enabled + +- use -r flag for claude backend resume + +- remove binary artifacts and improve error messages + +- 异常退出时显示最近错误信息 + +- op_run_command 实时流式输出 + +- 修复权限标志逻辑和版本号测试 + +- 重构信号处理逻辑避免重复 nil 检查 + +- 移除 .claude 配置文件验证步骤 + +- 修复并行执行启动横幅重复打印问题 + +- 修复master合并后的编译和测试问题 + +### 💼 Other + + +- Merge rc/5.2 into master: v5.2.0 release improvements + +- Merge pull request #53 from cexll/rc/5.2 + +- remove docs + +- remove docs + +- add prototype prompt skill + +- add prd skill + +- update memory claude + +- remove command gh flow + +- update license + +- Merge branch 'master' into rc/5.2 + +- Merge pull request #52 from cexll/fix/parallel-log-path-on-startup ### 📚 Documentation -- `docs/architecture.md` (21KB): Architecture overview with ASCII diagrams -- `docs/CODEAGENT-WRAPPER.md` (9KB): Complete usage guide -- `docs/HOOKS.md` (4KB): Customization guide -- `README.md`: Added documentation index, corrected default backend description -### 🔧 Important Fixes +- remove GitHub workflow related content -#### codeagent-wrapper -- Fixed Claude/Gemini backend `-C` (workdir) and `-r` (resume) parameter support (codeagent-wrapper/backend.go:80-120) -- Corrected Claude backend permission flag logic `if cfg.SkipPermissions` (codeagent-wrapper/backend.go:95) -- Fixed parallel mode startup banner duplication (codeagent-wrapper/main.go:184-194 removed) -- Extract and display recent errors on abnormal exit `Logger.ExtractRecentErrors()` (codeagent-wrapper/logger.go:156) -- Added task block index to parallel config error messages (codeagent-wrapper/config.go:245) -- Refactored signal handling logic to avoid duplicate nil checks (codeagent-wrapper/main.go:290-305) -- Removed binary artifacts from tracking (codeagent-wrapper, *.test, coverage.out) +### 🚀 Features -#### Installation Scripts -- Fixed issue #55: `op_run_command` uses Popen + selectors for real-time streaming output -- Fixed issue #56: Display recent errors instead of entire log -- Changed module list header from "Enabled" to "Default" to avoid ambiguity -#### CI/CD -- Removed `.claude/` config file validation step (.github/workflows/ci.yml:45) -- Updated version test case from 5.1.0 → 5.2.0 (codeagent-wrapper/main_test.go:23) +- Complete skills system integration and config cleanup -#### Commands & Documentation -- Reverted `skills/codex/SKILL.md` to `codex-wrapper` for backward compatibility +- Improve release notes and installation scripts -#### dev-workflow -- Replaced Codex skill → codeagent skill throughout -- Added UI auto-detection: backend tasks use codex, UI tasks use gemini -- Corrected agent name: `develop-doc-generator` → `dev-plan-generator` +- 添加终端日志输出和 verbose 模式 -### ⚙️ Configuration & Environment Variables +- 完整多后端支持与安全优化 -#### New Environment Variables -- `CODEAGENT_SKIP_PERMISSIONS`: Control permission check behavior - - Claude backend defaults to `--dangerously-skip-permissions` enabled, set to `true` to disable - - Codex/Gemini backends default to permission checks enabled, set to `true` to skip -- `CODEAGENT_MAX_PARALLEL_WORKERS`: Parallel task concurrency limit (default: unlimited, recommended: 8, max: 100) +- 替换 Codex 为 codeagent 并添加 UI 自动检测 -#### Configuration Files -- `config.schema.json`: Added `op_merge_json` schema validation +### 🚜 Refactor -### ⚠️ Breaking Changes -**codex-wrapper → codeagent-wrapper rename** +- 调整文件命名和技能定义 -**Migration**: -```bash -python3 install.py --module dev --force -``` +### 🧪 Testing -**Backward Compatibility**: `codex-wrapper/main.go` provides compatibility entry point -### 📦 Installation +- 添加 ExtractRecentErrors 单元测试 -```bash -# Install dev module -python3 install.py --module dev +## [5.1.4] - 2025-12-09 -# List all modules -python3 install.py --list-modules -# Verbose logging mode -python3 install.py --module dev --verbose -``` +### 🐛 Bug Fixes -### 🧪 Test Results -✅ **All tests passing** -- Overall coverage: 93.4% -- Security scan: 0 issues (gosec) -- Linting: Pass +- 任务启动时立即返回日志文件路径以支持实时调试 -### 📄 Related PRs & Issues +## [5.1.3] - 2025-12-08 -- PR #53: Enterprise Workflow with Multi-Backend Support -- Issue #55: Installation script execution not visible -- Issue #56: Unfriendly error logging on abnormal exit -### 👥 Contributors +### 🐛 Bug Fixes -- Claude Sonnet 4.5 -- Claude Opus 4.5 -- SWE-Agent-Bot + +- resolve CI timing race in TestFakeCmdInfra + +## [5.1.2] - 2025-12-08 + + +### 🐛 Bug Fixes + + +- 修复channel同步竞态条件和死锁问题 + +### 💼 Other + + +- Merge pull request #51 from cexll/fix/channel-sync-race-conditions + +- change codex-wrapper version + +## [5.1.1] - 2025-12-08 + + +### 🐛 Bug Fixes + + +- 增强日志清理的安全性和可靠性 + +- resolve data race on forceKillDelay with atomic operations + +### 💼 Other + + +- Merge pull request #49 from cexll/freespace8/master + +- resolve signal handling conflict preserving testability and Windows support + +### 🧪 Testing + + +- 补充测试覆盖提升至 89.3% + +## [5.1.0] - 2025-12-07 + + +### 💼 Other + + +- Merge pull request #45 from Michaelxwb/master + +- 修改windows安装说明 + +- 修改打包脚本 + +- 支持windows系统的安装 + +- Merge pull request #1 from Michaelxwb/feature-win + +- 支持window + +### 🚀 Features + + +- 添加启动时清理日志的功能和--cleanup标志支持 + +- implement enterprise workflow with multi-backend support + +## [5.0.0] - 2025-12-05 + + +### ⚙️ Miscellaneous Tasks + + +- clarify unit-test coverage levels in requirement questions + +### 🐛 Bug Fixes + + +- defer startup log until args parsed + +### 💼 Other + + +- Merge branch 'master' of github.com:cexll/myclaude + +- Merge pull request #43 from gurdasnijor/smithery/add-badge + +- Add Smithery badge + +- Merge pull request #42 from freespace8/master + +### 📚 Documentation + + +- rewrite documentation for v5.0 modular architecture + +### 🚀 Features + + +- feat install.py + +- implement modular installation system + +### 🚜 Refactor + + +- remove deprecated plugin modules + +## [4.8.2] - 2025-12-02 + + +### 🐛 Bug Fixes + + +- skip signal test in CI environment + +- make forceKillDelay testable to prevent signal test timeout + +- correct Go version in go.mod from 1.25.3 to 1.21 + +- fix codex wrapper async log + +- capture and include stderr in error messages + +### 💼 Other + + +- Merge pull request #41 from cexll/fix-async-log + +- remove test case 90 + +- optimize codex-wrapper + +- Merge branch 'master' into fix-async-log + +## [4.8.1] - 2025-12-01 + + +### 🎨 Styling + + +- replace emoji with text labels + +### 🐛 Bug Fixes + + +- improve --parallel parameter validation and docs + +### 💼 Other + + +- remove codex-wrapper bin + +## [4.8.0] - 2025-11-30 + + +### 💼 Other + + +- update codex skill dependencies + +## [4.7.3] - 2025-11-29 + + +### 🐛 Bug Fixes + + +- 保留日志文件以便程序退出后调试并完善日志输出功能 + +### 💼 Other + + +- Merge pull request #34 from cexll/cce-worktree-master-20251129-111802-997076000 + +- update CLAUDE.md and codex skill + +### 📚 Documentation + + +- improve codex skill parameter best practices + +### 🚀 Features + + +- add session resume support and improve output format + +- add parallel execution support to codex-wrapper + +- add async logging to temp file with lifecycle management + +## [4.7.2] - 2025-11-28 + + +### 🐛 Bug Fixes + + +- improve buffer size and streamline message extraction + +### 💼 Other + + +- Merge pull request #32 from freespace8/master + +### 🧪 Testing + + +- 增加对超大单行文本和非字符串文本的处理测试 + +## [4.7.1] - 2025-11-27 + + +### 💼 Other + + +- optimize dev pipline + +- Merge feat/codex-wrapper: fix repository URLs + +## [4.7] - 2025-11-27 + + +### 🐛 Bug Fixes + + +- update repository URLs to cexll/myclaude + +## [4.7-alpha1] - 2025-11-27 + + +### 🐛 Bug Fixes + + +- fix marketplace schema validation error in dev-workflow plugin + +### 💼 Other + + +- Merge pull request #29 from cexll/feat/codex-wrapper + +- Add codex-wrapper Go implementation + +- update readme + +- update readme + +## [4.6] - 2025-11-25 + + +### 💼 Other + + +- update dev workflow + +- update dev workflow + +## [4.5] - 2025-11-25 + + +### 🐛 Bug Fixes + + +- fix codex skill eof + +### 💼 Other + + +- update dev workflow plugin + +- update readme + +## [4.4] - 2025-11-22 + + +### 🐛 Bug Fixes + + +- fix codex skill timeout and add more log + +- fix codex skill + +### 💼 Other + + +- update gemini skills + +- update dev workflow + +- update codex skills model config + +- Merge branch 'master' of github.com:cexll/myclaude + +- Merge pull request #24 from cexll/swe-agent/23-1763544297 + +### 🚀 Features + + +- 支持通过环境变量配置 skills 模型 + +## [4.3] - 2025-11-19 + + +### 🐛 Bug Fixes + + +- fix codex skills running + +### 💼 Other + + +- update skills plugin + +- update gemini + +- update doc + +- Add Gemini CLI integration skill + +### 🚀 Features + + +- feat simple dev workflow + +## [4.2.2] - 2025-11-15 + + +### 💼 Other + + +- update codex skills + +## [4.2.1] - 2025-11-14 + + +### 💼 Other + + +- Merge pull request #21 from Tshoiasc/master + +- Merge branch 'master' into master + +- Change default model to gpt-5.1-codex + +- Enhance codex.py to auto-detect long inputs and switch to stdin mode, improving handling of shell argument issues. Updated build_codex_args to support stdin and added relevant logging for task length warnings. + +## [4.2] - 2025-11-13 + + +### 🐛 Bug Fixes + + +- fix codex.py wsl run err + +### 💼 Other + + +- optimize codex skills + +- Merge branch 'master' of github.com:cexll/myclaude + +- Rename SKILLS.md to SKILL.md + +- optimize codex skills + +### 🚀 Features + + +- feat codex skills + +## [4.1] - 2025-11-04 + + +### 💼 Other + + +- update enhance-prompt.md response + +- update readme + +### 📚 Documentation + + +- 新增 /enhance-prompt 命令并更新所有 README 文档 + +## [4.0] - 2025-10-22 + + +### 🐛 Bug Fixes + + +- fix skills format + +### 💼 Other + + +- Merge branch 'master' of github.com:cexll/myclaude + +- Merge pull request #18 from cexll/swe-agent/17-1760969135 + +- update requirements clarity + +- update .gitignore + +- Fix #17: Update root marketplace.json to use skills array + +- Fix #17: Convert requirements-clarity to correct plugin directory format + +- Fix #17: Convert requirements-clarity to correct plugin directory format + +- Convert requirements-clarity to plugin format with English prompts + +- Translate requirements-clarity skill to English for plugin compatibility + +- Add requirements-clarity Claude Skill + +- Add requirements clarification command + +- update + +## [3.5] - 2025-10-20 + + +### 💼 Other + + +- Merge pull request #15 from cexll/swe-agent/13-1760944712 + +- Fix #13: Clean up redundant README files + +- Optimize README structure - Solution A (modular) + +- Merge pull request #14 from cexll/swe-agent/12-1760944588 + +- Fix #12: Update Makefile install paths for new directory structure + +## [3.4] - 2025-10-20 + + +### 💼 Other + + +- Merge pull request #11 from cexll/swe-agent/10-1760752533 + +- Fix marketplace metadata references + +- Fix plugin configuration: rename to marketplace.json and update repository URLs + +- Fix #10: Restructure plugin directories to ensure proper command isolation + +## [3.3] - 2025-10-15 + + +### 💼 Other + + +- Update README-zh.md + +- Update README.md + +- Update marketplace.json + +- Update Chinese README with v3.2 plugin system documentation + +- Update README with v3.2 plugin system documentation + +## [3.2] - 2025-10-10 + + +### 💼 Other + + +- Add Claude Code plugin system support + +- update readme + +- Add Makefile for quick deployment and update READMEs + +## [3.1] - 2025-09-17 + + +### ◀️ Revert + + +- revert + +### 🐛 Bug Fixes + + +- fixed bmad-orchestrator not fund + +- fix bmad + +### 💼 Other + + +- update bmad review with codex support + +- 优化 BMAD 工作流和代理配置 + +- update gpt5 + +- support bmad output-style + +- update bmad user guide + +- update bmad readme + +- optimize requirements pilot + +- add use gpt5 codex + +- add bmad pilot + +- sync READMEs with actual commands/agents; remove nonexistent commands; enhance requirements-pilot with testing decision gate and options. + +- Update Chinese README and requirements-pilot command to align with latest workflow + +- update readme + +- update agent + +- update bugfix sub agents + +- Update ask support KISS YAGNI SOLID + +- Add comprehensive documentation and multi-agent workflow system + +- update commands + diff --git a/Makefile b/Makefile index 9fc260e..557f4ac 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # Claude Code Multi-Agent Workflow System Makefile # Quick deployment for BMAD and Requirements workflows -.PHONY: help install deploy-bmad deploy-requirements deploy-essentials deploy-advanced deploy-all deploy-commands deploy-agents clean test +.PHONY: help install deploy-bmad deploy-requirements deploy-essentials deploy-advanced deploy-all deploy-commands deploy-agents clean test changelog # Default target help: @@ -22,6 +22,7 @@ help: @echo " deploy-all - Deploy everything (commands + agents)" @echo " test-bmad - Test BMAD workflow with sample" @echo " test-requirements - Test Requirements workflow with sample" + @echo " changelog - Update CHANGELOG.md using git-cliff" @echo " clean - Clean generated artifacts" @echo " help - Show this help message" @@ -145,3 +146,17 @@ all: deploy-all version: @echo "Claude Code Multi-Agent Workflow System v3.1" @echo "BMAD + Requirements-Driven Development" + +# Update CHANGELOG.md using git-cliff +changelog: + @echo "📝 Updating CHANGELOG.md with git-cliff..." + @if ! command -v git-cliff > /dev/null 2>&1; then \ + echo "❌ git-cliff not found. Installing via Homebrew..."; \ + brew install git-cliff; \ + fi + @git-cliff -o CHANGELOG.md + @echo "✅ CHANGELOG.md updated successfully!" + @echo "" + @echo "Preview the changes:" + @echo " git diff CHANGELOG.md" + diff --git a/README.md b/README.md index d367029..978b51b 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ [![License: AGPL-3.0](https://img.shields.io/badge/License-AGPL_v3-blue.svg)](https://www.gnu.org/licenses/agpl-3.0) [![Claude Code](https://img.shields.io/badge/Claude-Code-blue)](https://claude.ai/code) -[![Version](https://img.shields.io/badge/Version-5.2.2-green)](https://github.com/cexll/myclaude) +[![Version](https://img.shields.io/badge/Version-5.2-green)](https://github.com/cexll/myclaude) > AI-powered development automation with multi-backend execution (Codex/Claude/Gemini) @@ -132,6 +132,59 @@ Requirements → Architecture → Sprint Plan → Development → Review → QA --- +## Version Requirements + +### Codex CLI +**Minimum version:** Check compatibility with your installation + +The codeagent-wrapper uses these Codex CLI features: +- `codex e` - Execute commands (shorthand for `codex exec`) +- `--skip-git-repo-check` - Skip git repository validation +- `--json` - JSON stream output format +- `-C ` - Set working directory +- `resume ` - Resume previous sessions + +**Verify Codex CLI is installed:** +```bash +which codex +codex --version +``` + +### Claude CLI +**Minimum version:** Check compatibility with your installation + +Required features: +- `--output-format stream-json` - Streaming JSON output format +- `--setting-sources` - Control setting sources (prevents infinite recursion) +- `--dangerously-skip-permissions` - Skip permission prompts (use with caution) +- `-p` - Prompt input flag +- `-r ` - Resume sessions + +**Security Note:** The wrapper only adds `--dangerously-skip-permissions` for Claude when explicitly enabled (e.g. `--skip-permissions` / `CODEAGENT_SKIP_PERMISSIONS=true`). Keep it disabled unless you understand the risk. + +**Verify Claude CLI is installed:** +```bash +which claude +claude --version +``` + +### Gemini CLI +**Minimum version:** Check compatibility with your installation + +Required features: +- `-o stream-json` - JSON stream output format +- `-y` - Auto-approve prompts (non-interactive mode) +- `-r ` - Resume sessions +- `-p` - Prompt input flag + +**Verify Gemini CLI is installed:** +```bash +which gemini +gemini --version +``` + +--- + ## Installation ### Modular Installation (Recommended) @@ -163,15 +216,39 @@ python3 install.py --force ``` ~/.claude/ -├── CLAUDE.md # Core instructions and role definition -├── commands/ # Slash commands (/dev, /code, etc.) -├── agents/ # Agent definitions +├── bin/ +│ └── codeagent-wrapper # Main executable +├── CLAUDE.md # Core instructions and role definition +├── commands/ # Slash commands (/dev, /code, etc.) +├── agents/ # Agent definitions ├── skills/ │ └── codex/ -│ └── SKILL.md # Codex integration skill -└── installed_modules.json # Installation status +│ └── SKILL.md # Codex integration skill +├── config.json # Configuration +└── installed_modules.json # Installation status ``` +### Customizing Installation Directory + +By default, myclaude installs to `~/.claude`. You can customize this using the `INSTALL_DIR` environment variable: + +```bash +# Install to custom directory +INSTALL_DIR=/opt/myclaude bash install.sh + +# Update your PATH accordingly +export PATH="/opt/myclaude/bin:$PATH" +``` + +**Directory Structure:** +- `$INSTALL_DIR/bin/` - codeagent-wrapper binary +- `$INSTALL_DIR/skills/` - Skill definitions +- `$INSTALL_DIR/config.json` - Configuration file +- `$INSTALL_DIR/commands/` - Slash command definitions +- `$INSTALL_DIR/agents/` - Agent definitions + +**Note:** When using a custom installation directory, ensure that `$INSTALL_DIR/bin` is added to your `PATH` environment variable. + ### Configuration Edit `config.json` to customize: @@ -294,11 +371,14 @@ setx PATH "%USERPROFILE%\bin;%PATH%" **Codex wrapper not found:** ```bash -# Check PATH -echo $PATH | grep -q "$HOME/bin" || echo 'export PATH="$HOME/bin:$PATH"' >> ~/.zshrc +# Installer auto-adds PATH, check if configured +if [[ ":$PATH:" != *":$HOME/.claude/bin:"* ]]; then + echo "PATH not configured. Reinstalling..." + bash install.sh +fi -# Reinstall -bash install.sh +# Or manually add (idempotent command) +[[ ":$PATH:" != *":$HOME/.claude/bin:"* ]] && echo 'export PATH="$HOME/.claude/bin:$PATH"' >> ~/.zshrc ``` **Permission denied:** @@ -315,6 +395,71 @@ cat ~/.claude/installed_modules.json python3 install.py --module dev --force ``` +### Version Compatibility Issues + +**Backend CLI not found:** +```bash +# Check if backend CLIs are installed +which codex +which claude +which gemini + +# Install missing backends +# Codex: Follow installation instructions at https://codex.docs +# Claude: Follow installation instructions at https://claude.ai/docs +# Gemini: Follow installation instructions at https://ai.google.dev/docs +``` + +**Unsupported CLI flags:** +```bash +# If you see errors like "unknown flag" or "invalid option" + +# Check backend CLI version +codex --version +claude --version +gemini --version + +# For Codex: Ensure it supports `e`, `--skip-git-repo-check`, `--json`, `-C`, and `resume` +# For Claude: Ensure it supports `--output-format stream-json`, `--setting-sources`, `-r` +# For Gemini: Ensure it supports `-o stream-json`, `-y`, `-r`, `-p` + +# Update your backend CLI to the latest version if needed +``` + +**JSON parsing errors:** +```bash +# If you see "failed to parse JSON output" errors + +# Verify the backend outputs stream-json format +codex e --json "test task" # Should output newline-delimited JSON +claude --output-format stream-json -p "test" # Should output stream JSON + +# If not, your backend CLI version may be too old or incompatible +``` + +**Infinite recursion with Claude backend:** +```bash +# The wrapper prevents this with `--setting-sources ""` flag +# If you still see recursion, ensure your Claude CLI supports this flag + +claude --help | grep "setting-sources" + +# If flag is not supported, upgrade Claude CLI +``` + +**Session resume failures:** +```bash +# Check if session ID is valid +codex history # List recent sessions +claude history + +# Ensure backend CLI supports session resumption +codex resume "test" # Should continue from previous session +claude -r "test" + +# If not supported, use new sessions instead of resume mode +``` + --- ## Documentation diff --git a/README_CN.md b/README_CN.md index 750e0ec..cdb85dc 100644 --- a/README_CN.md +++ b/README_CN.md @@ -2,7 +2,7 @@ [![License: AGPL-3.0](https://img.shields.io/badge/License-AGPL_v3-blue.svg)](https://www.gnu.org/licenses/agpl-3.0) [![Claude Code](https://img.shields.io/badge/Claude-Code-blue)](https://claude.ai/code) -[![Version](https://img.shields.io/badge/Version-5.2.2-green)](https://github.com/cexll/myclaude) +[![Version](https://img.shields.io/badge/Version-5.2-green)](https://github.com/cexll/myclaude) > AI 驱动的开发自动化 - 多后端执行架构 (Codex/Claude/Gemini) @@ -152,15 +152,39 @@ python3 install.py --force ``` ~/.claude/ -├── CLAUDE.md # 核心指令和角色定义 -├── commands/ # 斜杠命令 (/dev, /code 等) -├── agents/ # 智能体定义 +├── bin/ +│ └── codeagent-wrapper # 主可执行文件 +├── CLAUDE.md # 核心指令和角色定义 +├── commands/ # 斜杠命令 (/dev, /code 等) +├── agents/ # 智能体定义 ├── skills/ │ └── codex/ -│ └── SKILL.md # Codex 集成技能 -└── installed_modules.json # 安装状态 +│ └── SKILL.md # Codex 集成技能 +├── config.json # 配置文件 +└── installed_modules.json # 安装状态 ``` +### 自定义安装目录 + +默认情况下，myclaude 安装到 `~/.claude`。您可以使用 `INSTALL_DIR` 环境变量自定义安装目录： + +```bash +# 安装到自定义目录 +INSTALL_DIR=/opt/myclaude bash install.sh + +# 相应更新您的 PATH +export PATH="/opt/myclaude/bin:$PATH" +``` + +**目录结构：** +- `$INSTALL_DIR/bin/` - codeagent-wrapper 可执行文件 +- `$INSTALL_DIR/skills/` - 技能定义 +- `$INSTALL_DIR/config.json` - 配置文件 +- `$INSTALL_DIR/commands/` - 斜杠命令定义 +- `$INSTALL_DIR/agents/` - 智能体定义 + +**注意：** 使用自定义安装目录时，请确保将 `$INSTALL_DIR/bin` 添加到您的 `PATH` 环境变量中。 + ### 配置 编辑 `config.json` 自定义： @@ -283,11 +307,14 @@ setx PATH "%USERPROFILE%\bin;%PATH%" **Codex wrapper 未找到：** ```bash -# 检查 PATH -echo $PATH | grep -q "$HOME/bin" || echo 'export PATH="$HOME/bin:$PATH"' >> ~/.zshrc +# 安装程序会自动添加 PATH，检查是否已添加 +if [[ ":$PATH:" != *":$HOME/.claude/bin:"* ]]; then + echo "PATH not configured. Reinstalling..." + bash install.sh +fi -# 重新安装 -bash install.sh +# 或手动添加（幂等性命令） +[[ ":$PATH:" != *":$HOME/.claude/bin:"* ]] && echo 'export PATH="$HOME/.claude/bin:$PATH"' >> ~/.zshrc ``` **权限被拒绝：** diff --git a/bmad-agile-workflow/agents/bmad-architect.md b/bmad-agile-workflow/agents/bmad-architect.md index 9f9b924..602a05f 100644 --- a/bmad-agile-workflow/agents/bmad-architect.md +++ b/bmad-agile-workflow/agents/bmad-architect.md @@ -427,6 +427,10 @@ Generate architecture document at `./.claude/specs/{feature_name}/02-system-arch ## Important Behaviors +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, REST, GraphQL, JWT, RBAC, etc.) in English; translate explanatory text only. + ### DO: - Start by reviewing and referencing the PRD - Present initial architecture based on requirements diff --git a/bmad-agile-workflow/agents/bmad-dev.md b/bmad-agile-workflow/agents/bmad-dev.md index bfeb4a2..a535211 100644 --- a/bmad-agile-workflow/agents/bmad-dev.md +++ b/bmad-agile-workflow/agents/bmad-dev.md @@ -419,6 +419,10 @@ logger.info('User created', { ## Important Implementation Rules +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, CRUD, JWT, SQL, etc.) in English; translate explanatory text only. + ### DO: - Follow architecture specifications exactly - Implement all acceptance criteria from PRD diff --git a/bmad-agile-workflow/agents/bmad-orchestrator.md b/bmad-agile-workflow/agents/bmad-orchestrator.md index 7558c48..a9d61e1 100644 --- a/bmad-agile-workflow/agents/bmad-orchestrator.md +++ b/bmad-agile-workflow/agents/bmad-orchestrator.md @@ -22,6 +22,10 @@ You are the BMAD Orchestrator. Your core focus is repository analysis, workflow - Consistency: ensure conventions and patterns discovered in scan are preserved downstream - Explicit handoffs: clearly document assumptions, risks, and integration points for other agents +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, PRD, Sprint, etc.) in English; translate explanatory text only. + ## UltraThink Repository Scan When asked to analyze the repository, follow this structure and return a clear, actionable summary. diff --git a/bmad-agile-workflow/agents/bmad-po.md b/bmad-agile-workflow/agents/bmad-po.md index 4c552a9..e7323c5 100644 --- a/bmad-agile-workflow/agents/bmad-po.md +++ b/bmad-agile-workflow/agents/bmad-po.md @@ -313,6 +313,10 @@ Generate PRD at `./.claude/specs/{feature_name}/01-product-requirements.md`: ## Important Behaviors +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, Sprint, PRD, KPI, MVP, etc.) in English; translate explanatory text only. + ### DO: - Start immediately with greeting and initial understanding - Show quality scores transparently diff --git a/bmad-agile-workflow/agents/bmad-qa.md b/bmad-agile-workflow/agents/bmad-qa.md index 834c738..6264ee3 100644 --- a/bmad-agile-workflow/agents/bmad-qa.md +++ b/bmad-agile-workflow/agents/bmad-qa.md @@ -478,6 +478,10 @@ module.exports = { ## Important Testing Rules +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, E2E, CI/CD, Mock, etc.) in English; translate explanatory text only. + ### DO: - Test all acceptance criteria from PRD - Cover happy path, edge cases, and error scenarios diff --git a/bmad-agile-workflow/agents/bmad-review.md b/bmad-agile-workflow/agents/bmad-review.md index 2b8df57..ea32067 100644 --- a/bmad-agile-workflow/agents/bmad-review.md +++ b/bmad-agile-workflow/agents/bmad-review.md @@ -45,3 +45,7 @@ You are an independent code review agent responsible for conducting reviews betw - Focus on actionable findings - Provide specific QA guidance - Use clear, parseable output format + +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, PRD, Sprint, etc.) in English; translate explanatory text only. diff --git a/bmad-agile-workflow/agents/bmad-sm.md b/bmad-agile-workflow/agents/bmad-sm.md index 0042634..945874f 100644 --- a/bmad-agile-workflow/agents/bmad-sm.md +++ b/bmad-agile-workflow/agents/bmad-sm.md @@ -351,6 +351,10 @@ So that [benefit] ## Important Behaviors +### Language Rules: +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (Sprint, Epic, Story, Backlog, Velocity, etc.) in English; translate explanatory text only. + ### DO: - Read both PRD and Architecture documents thoroughly - Create comprehensive task breakdown diff --git a/cliff.toml b/cliff.toml new file mode 100644 index 0000000..186b251 --- /dev/null +++ b/cliff.toml @@ -0,0 +1,72 @@ +# git-cliff configuration file +# https://git-cliff.org/docs/configuration + +[changelog] +# changelog header +header = """ +# Changelog + +All notable changes to this project will be documented in this file. +""" +# template for the changelog body +body = """ +{% if version %} +## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }} +{% else %} +## Unreleased +{% endif %} +{% for group, commits in commits | group_by(attribute="group") %} +### {{ group }} + +{% for commit in commits %} +- {{ commit.message | split(pat="\n") | first }} +{% endfor -%} +{% endfor -%} +""" +# remove the leading and trailing whitespace from the template +trim = true +# changelog footer +footer = """ + +""" + +[git] +# parse the commits based on https://www.conventionalcommits.org +conventional_commits = true +# filter out the commits that are not conventional +filter_unconventional = false +# process each line of a commit as an individual commit +split_commits = false +# regex for preprocessing the commit messages +commit_preprocessors = [ + { pattern = '\((\w+\s)?#([0-9]+)\)', replace = "([#${2}](https://github.com/cexll/myclaude/issues/${2}))" }, +] +# regex for parsing and grouping commits +commit_parsers = [ + { message = "^feat", group = "🚀 Features" }, + { message = "^fix", group = "🐛 Bug Fixes" }, + { message = "^doc", group = "📚 Documentation" }, + { message = "^perf", group = "⚡ Performance" }, + { message = "^refactor", group = "🚜 Refactor" }, + { message = "^style", group = "🎨 Styling" }, + { message = "^test", group = "🧪 Testing" }, + { message = "^chore\\(release\\):", skip = true }, + { message = "^chore", group = "⚙️ Miscellaneous Tasks" }, + { body = ".*security", group = "🛡️ Security" }, + { message = "^revert", group = "◀️ Revert" }, + { message = ".*", group = "💼 Other" }, +] +# protect breaking changes from being skipped due to matching a skipping commit_parser +protect_breaking_commits = false +# filter out the commits that are not matched by commit parsers +filter_commits = false +# glob pattern for matching git tags +tag_pattern = "v[0-9]*" +# regex for skipping tags +skip_tags = "v0.1.0-beta.1" +# regex for ignoring tags +ignore_tags = "" +# sort the tags topologically +topo_order = false +# sort the commits inside sections by oldest/newest order +sort_commits = "newest" diff --git a/codeagent-wrapper/backend.go b/codeagent-wrapper/backend.go index 3ae9653..bcb6ecf 100644 --- a/codeagent-wrapper/backend.go +++ b/codeagent-wrapper/backend.go @@ -1,5 +1,11 @@ package main +import ( + "encoding/json" + "os" + "path/filepath" +) + // Backend defines the contract for invoking different AI CLI backends. // Each backend is responsible for supplying the executable command and // building the argument list based on the wrapper config. @@ -26,15 +32,66 @@ func (ClaudeBackend) Command() string { return "claude" } func (ClaudeBackend) BuildArgs(cfg *Config, targetArg string) []string { + return buildClaudeArgs(cfg, targetArg) +} + +const maxClaudeSettingsBytes = 1 << 20 // 1MB + +// loadMinimalEnvSettings 从 ~/.claude/settings.json 只提取 env 配置。 +// 只接受字符串类型的值；文件缺失/解析失败/超限都返回空。 +func loadMinimalEnvSettings() map[string]string { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return nil + } + + settingPath := filepath.Join(home, ".claude", "settings.json") + info, err := os.Stat(settingPath) + if err != nil || info.Size() > maxClaudeSettingsBytes { + return nil + } + + data, err := os.ReadFile(settingPath) + if err != nil { + return nil + } + + var cfg struct { + Env map[string]any `json:"env"` + } + if err := json.Unmarshal(data, &cfg); err != nil { + return nil + } + if len(cfg.Env) == 0 { + return nil + } + + env := make(map[string]string, len(cfg.Env)) + for k, v := range cfg.Env { + s, ok := v.(string) + if !ok { + continue + } + env[k] = s + } + if len(env) == 0 { + return nil + } + return env +} + +func buildClaudeArgs(cfg *Config, targetArg string) []string { if cfg == nil { return nil } - args := []string{"-p", "--dangerously-skip-permissions"} + args := []string{"-p"} + if cfg.SkipPermissions { + args = append(args, "--dangerously-skip-permissions") + } - // Only skip permissions when explicitly requested - // if cfg.SkipPermissions { - // args = append(args, "--dangerously-skip-permissions") - // } + // Prevent infinite recursion: disable all setting sources (user, project, local) + // This ensures a clean execution environment without CLAUDE.md or skills that would trigger codeagent + args = append(args, "--setting-sources", "") if cfg.Mode == "resume" { if cfg.SessionID != "" { @@ -56,6 +113,10 @@ func (GeminiBackend) Command() string { return "gemini" } func (GeminiBackend) BuildArgs(cfg *Config, targetArg string) []string { + return buildGeminiArgs(cfg, targetArg) +} + +func buildGeminiArgs(cfg *Config, targetArg string) []string { if cfg == nil { return nil } diff --git a/codeagent-wrapper/backend_test.go b/codeagent-wrapper/backend_test.go index dbe26f9..92faf1b 100644 --- a/codeagent-wrapper/backend_test.go +++ b/codeagent-wrapper/backend_test.go @@ -1,6 +1,9 @@ package main import ( + "bytes" + "os" + "path/filepath" "reflect" "testing" ) @@ -8,28 +11,28 @@ import ( func TestClaudeBuildArgs_ModesAndPermissions(t *testing.T) { backend := ClaudeBackend{} - t.Run("new mode uses workdir without skip by default", func(t *testing.T) { + t.Run("new mode omits skip-permissions by default", func(t *testing.T) { cfg := &Config{Mode: "new", WorkDir: "/repo"} got := backend.BuildArgs(cfg, "todo") - want := []string{"-p", "--dangerously-skip-permissions", "--output-format", "stream-json", "--verbose", "todo"} + want := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose", "todo"} if !reflect.DeepEqual(got, want) { t.Fatalf("got %v, want %v", got, want) } }) - t.Run("new mode opt-in skip permissions with default workdir", func(t *testing.T) { + t.Run("new mode can opt-in skip-permissions", func(t *testing.T) { cfg := &Config{Mode: "new", SkipPermissions: true} got := backend.BuildArgs(cfg, "-") - want := []string{"-p", "--dangerously-skip-permissions", "--output-format", "stream-json", "--verbose", "-"} + want := []string{"-p", "--dangerously-skip-permissions", "--setting-sources", "", "--output-format", "stream-json", "--verbose", "-"} if !reflect.DeepEqual(got, want) { t.Fatalf("got %v, want %v", got, want) } }) - t.Run("resume mode uses session id and omits workdir", func(t *testing.T) { + t.Run("resume mode includes session id", func(t *testing.T) { cfg := &Config{Mode: "resume", SessionID: "sid-123", WorkDir: "/ignored"} got := backend.BuildArgs(cfg, "resume-task") - want := []string{"-p", "--dangerously-skip-permissions", "-r", "sid-123", "--output-format", "stream-json", "--verbose", "resume-task"} + want := []string{"-p", "--setting-sources", "", "-r", "sid-123", "--output-format", "stream-json", "--verbose", "resume-task"} if !reflect.DeepEqual(got, want) { t.Fatalf("got %v, want %v", got, want) } @@ -38,7 +41,16 @@ func TestClaudeBuildArgs_ModesAndPermissions(t *testing.T) { t.Run("resume mode without session still returns base flags", func(t *testing.T) { cfg := &Config{Mode: "resume", WorkDir: "/ignored"} got := backend.BuildArgs(cfg, "follow-up") - want := []string{"-p", "--dangerously-skip-permissions", "--output-format", "stream-json", "--verbose", "follow-up"} + want := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose", "follow-up"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %v, want %v", got, want) + } + }) + + t.Run("resume mode can opt-in skip permissions", func(t *testing.T) { + cfg := &Config{Mode: "resume", SessionID: "sid-123", SkipPermissions: true} + got := backend.BuildArgs(cfg, "resume-task") + want := []string{"-p", "--dangerously-skip-permissions", "--setting-sources", "", "-r", "sid-123", "--output-format", "stream-json", "--verbose", "resume-task"} if !reflect.DeepEqual(got, want) { t.Fatalf("got %v, want %v", got, want) } @@ -89,7 +101,11 @@ func TestClaudeBuildArgs_GeminiAndCodexModes(t *testing.T) { } }) - t.Run("codex build args passthrough remains intact", func(t *testing.T) { + t.Run("codex build args omits bypass flag by default", func(t *testing.T) { + const key = "CODEX_BYPASS_SANDBOX" + t.Cleanup(func() { os.Unsetenv(key) }) + os.Unsetenv(key) + backend := CodexBackend{} cfg := &Config{Mode: "new", WorkDir: "/tmp"} got := backend.BuildArgs(cfg, "task") @@ -98,6 +114,20 @@ func TestClaudeBuildArgs_GeminiAndCodexModes(t *testing.T) { t.Fatalf("got %v, want %v", got, want) } }) + + t.Run("codex build args includes bypass flag when enabled", func(t *testing.T) { + const key = "CODEX_BYPASS_SANDBOX" + t.Cleanup(func() { os.Unsetenv(key) }) + os.Setenv(key, "true") + + backend := CodexBackend{} + cfg := &Config{Mode: "new", WorkDir: "/tmp"} + got := backend.BuildArgs(cfg, "task") + want := []string{"e", "--dangerously-bypass-approvals-and-sandbox", "--skip-git-repo-check", "-C", "/tmp", "--json", "task"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %v, want %v", got, want) + } + }) } func TestClaudeBuildArgs_BackendMetadata(t *testing.T) { @@ -120,3 +150,64 @@ func TestClaudeBuildArgs_BackendMetadata(t *testing.T) { } } } + +func TestLoadMinimalEnvSettings(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + t.Run("missing file returns empty", func(t *testing.T) { + if got := loadMinimalEnvSettings(); len(got) != 0 { + t.Fatalf("got %v, want empty", got) + } + }) + + t.Run("valid env returns string map", func(t *testing.T) { + dir := filepath.Join(home, ".claude") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + path := filepath.Join(dir, "settings.json") + data := []byte(`{"env":{"ANTHROPIC_API_KEY":"secret","FOO":"bar"}}`) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + got := loadMinimalEnvSettings() + if got["ANTHROPIC_API_KEY"] != "secret" || got["FOO"] != "bar" { + t.Fatalf("got %v, want keys present", got) + } + }) + + t.Run("non-string values are ignored", func(t *testing.T) { + dir := filepath.Join(home, ".claude") + path := filepath.Join(dir, "settings.json") + data := []byte(`{"env":{"GOOD":"ok","BAD":123,"ALSO_BAD":true}}`) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + got := loadMinimalEnvSettings() + if got["GOOD"] != "ok" { + t.Fatalf("got %v, want GOOD=ok", got) + } + if _, ok := got["BAD"]; ok { + t.Fatalf("got %v, want BAD omitted", got) + } + if _, ok := got["ALSO_BAD"]; ok { + t.Fatalf("got %v, want ALSO_BAD omitted", got) + } + }) + + t.Run("oversized file returns empty", func(t *testing.T) { + dir := filepath.Join(home, ".claude") + path := filepath.Join(dir, "settings.json") + data := bytes.Repeat([]byte("a"), maxClaudeSettingsBytes+1) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if got := loadMinimalEnvSettings(); len(got) != 0 { + t.Fatalf("got %v, want empty", got) + } + }) +} diff --git a/codeagent-wrapper/concurrent_stress_test.go b/codeagent-wrapper/concurrent_stress_test.go index 10fcc1e..822289a 100644 --- a/codeagent-wrapper/concurrent_stress_test.go +++ b/codeagent-wrapper/concurrent_stress_test.go @@ -13,6 +13,16 @@ import ( "time" ) +func stripTimestampPrefix(line string) string { + if !strings.HasPrefix(line, "[") { + return line + } + if idx := strings.Index(line, "] "); idx >= 0 { + return line[idx+2:] + } + return line +} + // TestConcurrentStressLogger 高并发压力测试 func TestConcurrentStressLogger(t *testing.T) { if testing.Short() { @@ -76,10 +86,11 @@ func TestConcurrentStressLogger(t *testing.T) { t.Logf("Successfully wrote %d/%d logs (%.1f%%)", actualCount, totalExpected, float64(actualCount)/float64(totalExpected)*100) - // 验证日志格式 - formatRE := regexp.MustCompile(`^\[\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3}\] \[PID:\d+\] INFO: goroutine-`) + // 验证日志格式（纯文本，无前缀） + formatRE := regexp.MustCompile(`^goroutine-\d+-msg-\d+$`) for i, line := range lines[:min(10, len(lines))] { - if !formatRE.MatchString(line) { + msg := stripTimestampPrefix(line) + if !formatRE.MatchString(msg) { t.Errorf("line %d has invalid format: %s", i, line) } } @@ -291,18 +302,15 @@ func TestLoggerOrderPreservation(t *testing.T) { sequences := make(map[int][]int) // goroutine ID -> sequence numbers for scanner.Scan() { - line := scanner.Text() + line := stripTimestampPrefix(scanner.Text()) var gid, seq int - parts := strings.SplitN(line, " INFO: ", 2) - if len(parts) != 2 { - t.Errorf("invalid log format: %s", line) + // Parse format: G0-SEQ0001 (without INFO: prefix) + _, err := fmt.Sscanf(line, "G%d-SEQ%04d", &gid, &seq) + if err != nil { + t.Errorf("invalid log format: %s (error: %v)", line, err) continue } - if _, err := fmt.Sscanf(parts[1], "G%d-SEQ%d", &gid, &seq); err == nil { - sequences[gid] = append(sequences[gid], seq) - } else { - t.Errorf("failed to parse sequence from line: %s", line) - } + sequences[gid] = append(sequences[gid], seq) } // 验证每个 goroutine 内部顺序 diff --git a/codeagent-wrapper/config.go b/codeagent-wrapper/config.go index bee3a2a..f7ad663 100644 --- a/codeagent-wrapper/config.go +++ b/codeagent-wrapper/config.go @@ -49,6 +49,15 @@ type TaskResult struct { SessionID string `json:"session_id"` Error string `json:"error"` LogPath string `json:"log_path"` + // Structured report fields + Coverage string `json:"coverage,omitempty"` // extracted coverage percentage (e.g., "92%") + CoverageNum float64 `json:"coverage_num,omitempty"` // numeric coverage for comparison + CoverageTarget float64 `json:"coverage_target,omitempty"` // target coverage (default 90) + FilesChanged []string `json:"files_changed,omitempty"` // list of changed files + KeyOutput string `json:"key_output,omitempty"` // brief summary of what was done + TestsPassed int `json:"tests_passed,omitempty"` // number of tests passed + TestsFailed int `json:"tests_failed,omitempty"` // number of tests failed + sharedLog bool } var backendRegistry = map[string]Backend{ @@ -163,6 +172,9 @@ func parseParallelConfig(data []byte) (*ParallelConfig, error) { if content == "" { return nil, fmt.Errorf("task block #%d (%q) missing content", taskIndex, task.ID) } + if task.Mode == "resume" && strings.TrimSpace(task.SessionID) == "" { + return nil, fmt.Errorf("task block #%d (%q) has empty session_id", taskIndex, task.ID) + } if _, exists := seen[task.ID]; exists { return nil, fmt.Errorf("task block #%d has duplicate id: %s", taskIndex, task.ID) } @@ -231,7 +243,10 @@ func parseArgs() (*Config, error) { return nil, fmt.Errorf("resume mode requires: resume ") } cfg.Mode = "resume" - cfg.SessionID = args[1] + cfg.SessionID = strings.TrimSpace(args[1]) + if cfg.SessionID == "" { + return nil, fmt.Errorf("resume mode requires non-empty session_id") + } cfg.Task = args[2] cfg.ExplicitStdin = (args[2] == "-") if len(args) > 3 { diff --git a/codeagent-wrapper/executor.go b/codeagent-wrapper/executor.go index d49a14f..db34887 100644 --- a/codeagent-wrapper/executor.go +++ b/codeagent-wrapper/executor.go @@ -16,6 +16,8 @@ import ( "time" ) +const postMessageTerminateDelay = 1 * time.Second + // commandRunner abstracts exec.Cmd for testability type commandRunner interface { Start() error @@ -24,6 +26,7 @@ type commandRunner interface { StdinPipe() (io.WriteCloser, error) SetStderr(io.Writer) SetDir(string) + SetEnv(env map[string]string) Process() processHandle } @@ -79,6 +82,52 @@ func (r *realCmd) SetDir(dir string) { } } +func (r *realCmd) SetEnv(env map[string]string) { + if r == nil || r.cmd == nil || len(env) == 0 { + return + } + + merged := make(map[string]string, len(env)+len(os.Environ())) + for _, kv := range os.Environ() { + if kv == "" { + continue + } + idx := strings.IndexByte(kv, '=') + if idx <= 0 { + continue + } + merged[kv[:idx]] = kv[idx+1:] + } + for _, kv := range r.cmd.Env { + if kv == "" { + continue + } + idx := strings.IndexByte(kv, '=') + if idx <= 0 { + continue + } + merged[kv[:idx]] = kv[idx+1:] + } + for k, v := range env { + if strings.TrimSpace(k) == "" { + continue + } + merged[k] = v + } + + keys := make([]string, 0, len(merged)) + for k := range merged { + keys = append(keys, k) + } + sort.Strings(keys) + + out := make([]string, 0, len(keys)) + for _, k := range keys { + out = append(out, k+"="+merged[k]) + } + r.cmd.Env = out +} + func (r *realCmd) Process() processHandle { if r == nil || r.cmd == nil || r.cmd.Process == nil { return nil @@ -122,7 +171,57 @@ type parseResult struct { threadID string } -var runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { +type taskLoggerContextKey struct{} + +func withTaskLogger(ctx context.Context, logger *Logger) context.Context { + if ctx == nil || logger == nil { + return ctx + } + return context.WithValue(ctx, taskLoggerContextKey{}, logger) +} + +func taskLoggerFromContext(ctx context.Context) *Logger { + if ctx == nil { + return nil + } + logger, _ := ctx.Value(taskLoggerContextKey{}).(*Logger) + return logger +} + +type taskLoggerHandle struct { + logger *Logger + path string + shared bool + closeFn func() +} + +func newTaskLoggerHandle(taskID string) taskLoggerHandle { + taskLogger, err := NewLoggerWithSuffix(taskID) + if err == nil { + return taskLoggerHandle{ + logger: taskLogger, + path: taskLogger.Path(), + closeFn: func() { _ = taskLogger.Close() }, + } + } + + msg := fmt.Sprintf("Failed to create task logger for %s: %v, using main logger", taskID, err) + mainLogger := activeLogger() + if mainLogger != nil { + logWarn(msg) + return taskLoggerHandle{ + logger: mainLogger, + path: mainLogger.Path(), + shared: true, + } + } + + fmt.Fprintln(os.Stderr, msg) + return taskLoggerHandle{} +} + +// defaultRunCodexTaskFn is the default implementation of runCodexTaskFn (exposed for test reset) +func defaultRunCodexTaskFn(task TaskSpec, timeout int) TaskResult { if task.WorkDir == "" { task.WorkDir = defaultWorkdir } @@ -151,6 +250,8 @@ var runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { return runCodexTaskWithContext(parentCtx, task, backend, nil, false, true, timeout) } +var runCodexTaskFn = defaultRunCodexTaskFn + func topologicalSort(tasks []TaskSpec) ([][]TaskSpec, error) { idToTask := make(map[string]TaskSpec, len(tasks)) indegree := make(map[string]int, len(tasks)) @@ -235,13 +336,8 @@ func executeConcurrentWithContext(parentCtx context.Context, layers [][]TaskSpec var startPrintMu sync.Mutex bannerPrinted := false - printTaskStart := func(taskID string) { - logger := activeLogger() - if logger == nil { - return - } - path := logger.Path() - if path == "" { + printTaskStart := func(taskID, logPath string, shared bool) { + if logPath == "" { return } startPrintMu.Lock() @@ -249,7 +345,11 @@ func executeConcurrentWithContext(parentCtx context.Context, layers [][]TaskSpec fmt.Fprintln(os.Stderr, "=== Starting Parallel Execution ===") bannerPrinted = true } - fmt.Fprintf(os.Stderr, "Task %s: Log: %s\n", taskID, path) + label := "Log" + if shared { + label = "Log (shared)" + } + fmt.Fprintf(os.Stderr, "Task %s: %s: %s\n", taskID, label, logPath) startPrintMu.Unlock() } @@ -319,9 +419,11 @@ func executeConcurrentWithContext(parentCtx context.Context, layers [][]TaskSpec wg.Add(1) go func(ts TaskSpec) { defer wg.Done() + var taskLogPath string + handle := taskLoggerHandle{} defer func() { if r := recover(); r != nil { - resultsCh <- TaskResult{TaskID: ts.ID, ExitCode: 1, Error: fmt.Sprintf("panic: %v", r)} + resultsCh <- TaskResult{TaskID: ts.ID, ExitCode: 1, Error: fmt.Sprintf("panic: %v", r), LogPath: taskLogPath, sharedLog: handle.shared} } }() @@ -338,9 +440,31 @@ func executeConcurrentWithContext(parentCtx context.Context, layers [][]TaskSpec logConcurrencyState("done", ts.ID, int(after), workerLimit) }() - ts.Context = ctx - printTaskStart(ts.ID) - resultsCh <- runCodexTaskFn(ts, timeout) + handle = newTaskLoggerHandle(ts.ID) + taskLogPath = handle.path + if handle.closeFn != nil { + defer handle.closeFn() + } + + taskCtx := ctx + if handle.logger != nil { + taskCtx = withTaskLogger(ctx, handle.logger) + } + ts.Context = taskCtx + + printTaskStart(ts.ID, taskLogPath, handle.shared) + + res := runCodexTaskFn(ts, timeout) + if taskLogPath != "" { + if res.LogPath == "" || (handle.shared && handle.logger != nil && res.LogPath == handle.logger.Path()) { + res.LogPath = taskLogPath + } + } + // 只有当最终的 LogPath 确实是共享 logger 的路径时才标记为 shared + if handle.shared && handle.logger != nil && res.LogPath == handle.logger.Path() { + res.sharedLog = true + } + resultsCh <- res }(task) } @@ -387,64 +511,255 @@ func shouldSkipTask(task TaskSpec, failed map[string]TaskResult) (bool, string) return true, fmt.Sprintf("skipped due to failed dependencies: %s", strings.Join(blocked, ",")) } -func generateFinalOutput(results []TaskResult) string { - var sb strings.Builder +// getStatusSymbols returns status symbols based on ASCII mode. +func getStatusSymbols() (success, warning, failed string) { + if os.Getenv("CODEAGENT_ASCII_MODE") == "true" { + return "PASS", "WARN", "FAIL" + } + return "✓", "⚠️", "✗" +} +func generateFinalOutput(results []TaskResult) string { + return generateFinalOutputWithMode(results, true) // default to summary mode +} + +// generateFinalOutputWithMode generates output based on mode +// summaryOnly=true: structured report - every token has value +// summaryOnly=false: full output with complete messages (legacy behavior) +func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string { + var sb strings.Builder + successSymbol, warningSymbol, failedSymbol := getStatusSymbols() + + reportCoverageTarget := defaultCoverageTarget + for _, res := range results { + if res.CoverageTarget > 0 { + reportCoverageTarget = res.CoverageTarget + break + } + } + + // Count results by status success := 0 failed := 0 + belowTarget := 0 for _, res := range results { if res.ExitCode == 0 && res.Error == "" { success++ + target := res.CoverageTarget + if target <= 0 { + target = reportCoverageTarget + } + if res.Coverage != "" && target > 0 && res.CoverageNum < target { + belowTarget++ + } } else { failed++ } } - sb.WriteString(fmt.Sprintf("=== Parallel Execution Summary ===\n")) - sb.WriteString(fmt.Sprintf("Total: %d | Success: %d | Failed: %d\n\n", len(results), success, failed)) + if summaryOnly { + // Header + sb.WriteString("=== Execution Report ===\n") + sb.WriteString(fmt.Sprintf("%d tasks | %d passed | %d failed", len(results), success, failed)) + if belowTarget > 0 { + sb.WriteString(fmt.Sprintf(" | %d below %.0f%%", belowTarget, reportCoverageTarget)) + } + sb.WriteString("\n\n") - for _, res := range results { - sb.WriteString(fmt.Sprintf("--- Task: %s ---\n", res.TaskID)) - if res.Error != "" { - sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\nError: %s\n", res.ExitCode, res.Error)) - } else if res.ExitCode != 0 { - sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\n", res.ExitCode)) - } else { - sb.WriteString("Status: SUCCESS\n") + // Task Results - each task gets: Did + Files + Tests + Coverage + sb.WriteString("## Task Results\n") + + for _, res := range results { + taskID := sanitizeOutput(res.TaskID) + coverage := sanitizeOutput(res.Coverage) + keyOutput := sanitizeOutput(res.KeyOutput) + logPath := sanitizeOutput(res.LogPath) + filesChanged := sanitizeOutput(strings.Join(res.FilesChanged, ", ")) + + target := res.CoverageTarget + if target <= 0 { + target = reportCoverageTarget + } + + isSuccess := res.ExitCode == 0 && res.Error == "" + isBelowTarget := isSuccess && coverage != "" && target > 0 && res.CoverageNum < target + + if isSuccess && !isBelowTarget { + // Passed task: one block with Did/Files/Tests + sb.WriteString(fmt.Sprintf("\n### %s %s", taskID, successSymbol)) + if coverage != "" { + sb.WriteString(fmt.Sprintf(" %s", coverage)) + } + sb.WriteString("\n") + + if keyOutput != "" { + sb.WriteString(fmt.Sprintf("Did: %s\n", keyOutput)) + } + if len(res.FilesChanged) > 0 { + sb.WriteString(fmt.Sprintf("Files: %s\n", filesChanged)) + } + if res.TestsPassed > 0 { + sb.WriteString(fmt.Sprintf("Tests: %d passed\n", res.TestsPassed)) + } + if logPath != "" { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) + } + + } else if isSuccess && isBelowTarget { + // Below target: add Gap info + sb.WriteString(fmt.Sprintf("\n### %s %s %s (below %.0f%%)\n", taskID, warningSymbol, coverage, target)) + + if keyOutput != "" { + sb.WriteString(fmt.Sprintf("Did: %s\n", keyOutput)) + } + if len(res.FilesChanged) > 0 { + sb.WriteString(fmt.Sprintf("Files: %s\n", filesChanged)) + } + if res.TestsPassed > 0 { + sb.WriteString(fmt.Sprintf("Tests: %d passed\n", res.TestsPassed)) + } + // Extract what's missing from coverage + gap := sanitizeOutput(extractCoverageGap(res.Message)) + if gap != "" { + sb.WriteString(fmt.Sprintf("Gap: %s\n", gap)) + } + if logPath != "" { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) + } + + } else { + // Failed task: show error detail + sb.WriteString(fmt.Sprintf("\n### %s %s FAILED\n", taskID, failedSymbol)) + sb.WriteString(fmt.Sprintf("Exit code: %d\n", res.ExitCode)) + if errText := sanitizeOutput(res.Error); errText != "" { + sb.WriteString(fmt.Sprintf("Error: %s\n", errText)) + } + // Show context from output (last meaningful lines) + detail := sanitizeOutput(extractErrorDetail(res.Message, 300)) + if detail != "" { + sb.WriteString(fmt.Sprintf("Detail: %s\n", detail)) + } + if logPath != "" { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) + } + } } - if res.SessionID != "" { - sb.WriteString(fmt.Sprintf("Session: %s\n", res.SessionID)) + + // Summary section + sb.WriteString("\n## Summary\n") + sb.WriteString(fmt.Sprintf("- %d/%d completed successfully\n", success, len(results))) + + if belowTarget > 0 || failed > 0 { + var needFix []string + var needCoverage []string + for _, res := range results { + if res.ExitCode != 0 || res.Error != "" { + taskID := sanitizeOutput(res.TaskID) + reason := sanitizeOutput(res.Error) + if reason == "" && res.ExitCode != 0 { + reason = fmt.Sprintf("exit code %d", res.ExitCode) + } + reason = safeTruncate(reason, 50) + needFix = append(needFix, fmt.Sprintf("%s (%s)", taskID, reason)) + continue + } + + target := res.CoverageTarget + if target <= 0 { + target = reportCoverageTarget + } + if res.Coverage != "" && target > 0 && res.CoverageNum < target { + needCoverage = append(needCoverage, sanitizeOutput(res.TaskID)) + } + } + if len(needFix) > 0 { + sb.WriteString(fmt.Sprintf("- Fix: %s\n", strings.Join(needFix, ", "))) + } + if len(needCoverage) > 0 { + sb.WriteString(fmt.Sprintf("- Coverage: %s\n", strings.Join(needCoverage, ", "))) + } } - if res.LogPath != "" { - sb.WriteString(fmt.Sprintf("Log: %s\n", res.LogPath)) + + } else { + // Legacy full output mode + sb.WriteString("=== Parallel Execution Summary ===\n") + sb.WriteString(fmt.Sprintf("Total: %d | Success: %d | Failed: %d\n\n", len(results), success, failed)) + + for _, res := range results { + taskID := sanitizeOutput(res.TaskID) + sb.WriteString(fmt.Sprintf("--- Task: %s ---\n", taskID)) + if res.Error != "" { + sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\nError: %s\n", res.ExitCode, sanitizeOutput(res.Error))) + } else if res.ExitCode != 0 { + sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\n", res.ExitCode)) + } else { + sb.WriteString("Status: SUCCESS\n") + } + if res.Coverage != "" { + sb.WriteString(fmt.Sprintf("Coverage: %s\n", sanitizeOutput(res.Coverage))) + } + if res.SessionID != "" { + sb.WriteString(fmt.Sprintf("Session: %s\n", sanitizeOutput(res.SessionID))) + } + if res.LogPath != "" { + logPath := sanitizeOutput(res.LogPath) + if res.sharedLog { + sb.WriteString(fmt.Sprintf("Log: %s (shared)\n", logPath)) + } else { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) + } + } + if res.Message != "" { + message := sanitizeOutput(res.Message) + if message != "" { + sb.WriteString(fmt.Sprintf("\n%s\n", message)) + } + } + sb.WriteString("\n") } - if res.Message != "" { - sb.WriteString(fmt.Sprintf("\n%s\n", res.Message)) - } - sb.WriteString("\n") } return sb.String() } func buildCodexArgs(cfg *Config, targetArg string) []string { - if cfg.Mode == "resume" { - return []string{ - "e", - "--skip-git-repo-check", - "--json", - "resume", - cfg.SessionID, - targetArg, + if cfg == nil { + panic("buildCodexArgs: nil config") + } + + var resumeSessionID string + isResume := cfg.Mode == "resume" + if isResume { + resumeSessionID = strings.TrimSpace(cfg.SessionID) + if resumeSessionID == "" { + logError("invalid config: resume mode requires non-empty session_id") + isResume = false } } - return []string{ - "e", - "--skip-git-repo-check", + + args := []string{"e"} + + if envFlagEnabled("CODEX_BYPASS_SANDBOX") { + logWarn("CODEX_BYPASS_SANDBOX=true: running without approval/sandbox protection") + args = append(args, "--dangerously-bypass-approvals-and-sandbox") + } + + args = append(args, "--skip-git-repo-check") + + if isResume { + return append(args, + "--json", + "resume", + resumeSessionID, + targetArg, + ) + } + + return append(args, "-C", cfg.WorkDir, "--json", targetArg, - } + ) } func runCodexTask(taskSpec TaskSpec, silent bool, timeoutSec int) TaskResult { @@ -457,15 +772,16 @@ func runCodexProcess(parentCtx context.Context, codexArgs []string, taskText str } func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backend Backend, customArgs []string, useCustomArgs bool, silent bool, timeoutSec int) TaskResult { - result := TaskResult{TaskID: taskSpec.ID} - setLogPath := func() { - if result.LogPath != "" { - return - } - if logger := activeLogger(); logger != nil { - result.LogPath = logger.Path() - } + if parentCtx == nil { + parentCtx = taskSpec.Context } + if parentCtx == nil { + parentCtx = context.Background() + } + + result := TaskResult{TaskID: taskSpec.ID} + injectedLogger := taskLoggerFromContext(parentCtx) + logger := injectedLogger cfg := &Config{ Mode: taskSpec.Mode, @@ -494,6 +810,12 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe cfg.WorkDir = defaultWorkdir } + if cfg.Mode == "resume" && strings.TrimSpace(cfg.SessionID) == "" { + result.ExitCode = 1 + result.Error = "resume mode requires non-empty session_id" + return result + } + useStdin := taskSpec.UseStdin targetArg := taskSpec.Task if useStdin { @@ -521,17 +843,17 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe if silent { // Silent mode: only persist to file when available; avoid stderr noise. logInfoFn = func(msg string) { - if logger := activeLogger(); logger != nil { + if logger != nil { logger.Info(prefixMsg(msg)) } } logWarnFn = func(msg string) { - if logger := activeLogger(); logger != nil { + if logger != nil { logger.Warn(prefixMsg(msg)) } } logErrorFn = func(msg string) { - if logger := activeLogger(); logger != nil { + if logger != nil { logger.Error(prefixMsg(msg)) } } @@ -547,10 +869,11 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe var stderrLogger *logWriter var tempLogger *Logger - if silent && activeLogger() == nil { + if logger == nil && silent && activeLogger() == nil { if l, err := NewLogger(); err == nil { setLogger(l) tempLogger = l + logger = l } } defer func() { @@ -558,21 +881,29 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe _ = closeLogger() } }() - defer setLogPath() - if logger := activeLogger(); logger != nil { + defer func() { + if result.LogPath != "" || logger == nil { + return + } + result.LogPath = logger.Path() + }() + if logger == nil { + logger = activeLogger() + } + if logger != nil { result.LogPath = logger.Path() } if !silent { - stdoutLogger = newLogWriter("CODEX_STDOUT: ", codexLogLineLimit) - stderrLogger = newLogWriter("CODEX_STDERR: ", codexLogLineLimit) + // Note: Empty prefix ensures backend output is logged as-is without any wrapper format. + // This preserves the original stdout/stderr content from codex/claude/gemini backends. + // Trade-off: Reduces distinguishability between stdout/stderr in logs, but maintains + // output fidelity which is critical for debugging backend-specific issues. + stdoutLogger = newLogWriter("", codexLogLineLimit) + stderrLogger = newLogWriter("", codexLogLineLimit) } ctx := parentCtx - if ctx == nil { - ctx = context.Background() - } - ctx, cancel := context.WithTimeout(ctx, time.Duration(timeoutSec)*time.Second) defer cancel() ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) @@ -584,6 +915,12 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe cmd := newCommandRunner(ctx, commandName, codexArgs...) + if cfg.Backend == "claude" { + if env := loadMinimalEnvSettings(); len(env) > 0 { + cmd.SetEnv(env) + } + } + // For backends that don't support -C flag (claude, gemini), set working directory via cmd.Dir // Codex passes workdir via -C flag, so we skip setting Dir for it to avoid conflicts if cfg.Mode != "resume" && commandName != "codex" && cfg.WorkDir != "" { @@ -594,8 +931,17 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe if stderrLogger != nil { stderrWriters = append(stderrWriters, stderrLogger) } + + // For gemini backend, filter noisy stderr output + var stderrFilter *filteringWriter if !silent { - stderrWriters = append([]io.Writer{os.Stderr}, stderrWriters...) + stderrOut := io.Writer(os.Stderr) + if cfg.Backend == "gemini" { + stderrFilter = newFilteringWriter(os.Stderr, geminiNoisePatterns) + stderrOut = stderrFilter + defer stderrFilter.Flush() + } + stderrWriters = append([]io.Writer{stderrOut}, stderrWriters...) } if len(stderrWriters) == 1 { cmd.SetStderr(stderrWriters[0]) @@ -631,6 +977,7 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe // Start parse goroutine BEFORE starting the command to avoid race condition // where fast-completing commands close stdout before parser starts reading messageSeen := make(chan struct{}, 1) + completeSeen := make(chan struct{}, 1) parseCh := make(chan parseResult, 1) go func() { msg, tid := parseJSONStreamInternal(stdoutReader, logWarnFn, logInfoFn, func() { @@ -638,7 +985,16 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe case messageSeen <- struct{}{}: default: } + }, func() { + select { + case completeSeen <- struct{}{}: + default: + } }) + select { + case completeSeen <- struct{}{}: + default: + } parseCh <- parseResult{message: msg, threadID: tid} }() @@ -659,7 +1015,7 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe } logInfoFn(fmt.Sprintf("Starting %s with PID: %d", commandName, cmd.Process().Pid())) - if logger := activeLogger(); logger != nil { + if logger != nil { logInfoFn(fmt.Sprintf("Log capturing to: %s", logger.Path())) } @@ -675,17 +1031,63 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe waitCh := make(chan error, 1) go func() { waitCh <- cmd.Wait() }() - var waitErr error - var forceKillTimer *forceKillTimer - var ctxCancelled bool + var ( + waitErr error + forceKillTimer *forceKillTimer + ctxCancelled bool + messageTimer *time.Timer + messageTimerCh <-chan time.Time + forcedAfterComplete bool + terminated bool + messageSeenObserved bool + completeSeenObserved bool + ) - select { - case waitErr = <-waitCh: - case <-ctx.Done(): - ctxCancelled = true - logErrorFn(cancelReason(commandName, ctx)) - forceKillTimer = terminateCommandFn(cmd) - waitErr = <-waitCh +waitLoop: + for { + select { + case waitErr = <-waitCh: + break waitLoop + case <-ctx.Done(): + ctxCancelled = true + logErrorFn(cancelReason(commandName, ctx)) + if !terminated { + if timer := terminateCommandFn(cmd); timer != nil { + forceKillTimer = timer + terminated = true + } + } + waitErr = <-waitCh + break waitLoop + case <-messageTimerCh: + forcedAfterComplete = true + messageTimerCh = nil + if !terminated { + logWarnFn(fmt.Sprintf("%s output parsed; terminating lingering backend", commandName)) + if timer := terminateCommandFn(cmd); timer != nil { + forceKillTimer = timer + terminated = true + } + } + case <-completeSeen: + completeSeenObserved = true + if messageTimer != nil { + continue + } + messageTimer = time.NewTimer(postMessageTerminateDelay) + messageTimerCh = messageTimer.C + case <-messageSeen: + messageSeenObserved = true + } + } + + if messageTimer != nil { + if !messageTimer.Stop() { + select { + case <-messageTimer.C: + default: + } + } } if forceKillTimer != nil { @@ -693,10 +1095,14 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe } var parsed parseResult - if ctxCancelled { + switch { + case ctxCancelled: closeWithReason(stdout, stdoutCloseReasonCtx) parsed = <-parseCh - } else { + case messageSeenObserved || completeSeenObserved: + closeWithReason(stdout, stdoutCloseReasonWait) + parsed = <-parseCh + default: drainTimer := time.NewTimer(stdoutDrainTimeout) defer drainTimer.Stop() @@ -704,6 +1110,11 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe case parsed = <-parseCh: closeWithReason(stdout, stdoutCloseReasonWait) case <-messageSeen: + messageSeenObserved = true + closeWithReason(stdout, stdoutCloseReasonWait) + parsed = <-parseCh + case <-completeSeen: + completeSeenObserved = true closeWithReason(stdout, stdoutCloseReasonWait) parsed = <-parseCh case <-drainTimer.C: @@ -724,17 +1135,21 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe } if waitErr != nil { - if exitErr, ok := waitErr.(*exec.ExitError); ok { - code := exitErr.ExitCode() - logErrorFn(fmt.Sprintf("%s exited with status %d", commandName, code)) - result.ExitCode = code - result.Error = attachStderr(fmt.Sprintf("%s exited with status %d", commandName, code)) + if forcedAfterComplete && parsed.message != "" { + logWarnFn(fmt.Sprintf("%s terminated after delivering output", commandName)) + } else { + if exitErr, ok := waitErr.(*exec.ExitError); ok { + code := exitErr.ExitCode() + logErrorFn(fmt.Sprintf("%s exited with status %d", commandName, code)) + result.ExitCode = code + result.Error = attachStderr(fmt.Sprintf("%s exited with status %d", commandName, code)) + return result + } + logErrorFn(commandName + " error: " + waitErr.Error()) + result.ExitCode = 1 + result.Error = attachStderr(commandName + " error: " + waitErr.Error()) return result } - logErrorFn(commandName + " error: " + waitErr.Error()) - result.ExitCode = 1 - result.Error = attachStderr(commandName + " error: " + waitErr.Error()) - return result } message := parsed.message @@ -756,8 +1171,8 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe result.ExitCode = 0 result.Message = message result.SessionID = threadID - if logger := activeLogger(); logger != nil { - result.LogPath = logger.Path() + if result.LogPath == "" && injectedLogger != nil { + result.LogPath = injectedLogger.Path() } return result diff --git a/codeagent-wrapper/executor_concurrent_test.go b/codeagent-wrapper/executor_concurrent_test.go index 4c56961..bc6fcbc 100644 --- a/codeagent-wrapper/executor_concurrent_test.go +++ b/codeagent-wrapper/executor_concurrent_test.go @@ -1,12 +1,16 @@ package main import ( + "bufio" "bytes" "context" "errors" + "fmt" "io" "os" "os/exec" + "path/filepath" + "slices" "strings" "sync" "sync/atomic" @@ -15,6 +19,12 @@ import ( "time" ) +var executorTestTaskCounter atomic.Int64 + +func nextExecutorTestTaskID(prefix string) string { + return fmt.Sprintf("%s-%d", prefix, executorTestTaskCounter.Add(1)) +} + type execFakeProcess struct { pid int signals []os.Signal @@ -76,6 +86,8 @@ type execFakeRunner struct { stdout io.ReadCloser process processHandle stdin io.WriteCloser + dir string + env map[string]string waitErr error waitDelay time.Duration startErr error @@ -117,7 +129,18 @@ func (f *execFakeRunner) StdinPipe() (io.WriteCloser, error) { return &writeCloserStub{}, nil } func (f *execFakeRunner) SetStderr(io.Writer) {} -func (f *execFakeRunner) SetDir(string) {} +func (f *execFakeRunner) SetDir(dir string) { f.dir = dir } +func (f *execFakeRunner) SetEnv(env map[string]string) { + if len(env) == 0 { + return + } + if f.env == nil { + f.env = make(map[string]string, len(env)) + } + for k, v := range env { + f.env[k] = v + } +} func (f *execFakeRunner) Process() processHandle { if f.process != nil { return f.process @@ -149,6 +172,10 @@ func TestExecutorHelperCoverage(t *testing.T) { } rcWithCmd := &realCmd{cmd: &exec.Cmd{}} rcWithCmd.SetStderr(io.Discard) + rcWithCmd.SetDir("/tmp") + if rcWithCmd.cmd.Dir != "/tmp" { + t.Fatalf("expected SetDir to set cmd.Dir, got %q", rcWithCmd.cmd.Dir) + } echoCmd := exec.Command("echo", "ok") rcProc := &realCmd{cmd: echoCmd} stdoutPipe, err := rcProc.StdoutPipe() @@ -230,6 +257,10 @@ func TestExecutorHelperCoverage(t *testing.T) { }) t.Run("generateFinalOutputAndArgs", func(t *testing.T) { + const key = "CODEX_BYPASS_SANDBOX" + t.Cleanup(func() { os.Unsetenv(key) }) + os.Unsetenv(key) + out := generateFinalOutput([]TaskResult{ {TaskID: "ok", ExitCode: 0}, {TaskID: "fail", ExitCode: 1, Error: "boom"}, @@ -237,21 +268,66 @@ func TestExecutorHelperCoverage(t *testing.T) { if !strings.Contains(out, "ok") || !strings.Contains(out, "fail") { t.Fatalf("unexpected summary output: %s", out) } + // Test summary mode (default) - should have new format with ### headers out = generateFinalOutput([]TaskResult{{TaskID: "rich", ExitCode: 0, SessionID: "sess", LogPath: "/tmp/log", Message: "hello"}}) + if !strings.Contains(out, "### rich") { + t.Fatalf("summary output missing task header: %s", out) + } + // Test full output mode - should have Session and Message + out = generateFinalOutputWithMode([]TaskResult{{TaskID: "rich", ExitCode: 0, SessionID: "sess", LogPath: "/tmp/log", Message: "hello"}}, false) if !strings.Contains(out, "Session: sess") || !strings.Contains(out, "Log: /tmp/log") || !strings.Contains(out, "hello") { - t.Fatalf("rich output missing fields: %s", out) + t.Fatalf("full output missing fields: %s", out) } args := buildCodexArgs(&Config{Mode: "new", WorkDir: "/tmp"}, "task") - if len(args) == 0 || args[3] != "/tmp" { + if !slices.Equal(args, []string{"e", "--skip-git-repo-check", "-C", "/tmp", "--json", "task"}) { t.Fatalf("unexpected codex args: %+v", args) } args = buildCodexArgs(&Config{Mode: "resume", SessionID: "sess"}, "target") - if args[3] != "resume" || args[4] != "sess" { + if !slices.Equal(args, []string{"e", "--skip-git-repo-check", "--json", "resume", "sess", "target"}) { t.Fatalf("unexpected resume args: %+v", args) } }) + t.Run("generateFinalOutputASCIIMode", func(t *testing.T) { + t.Setenv("CODEAGENT_ASCII_MODE", "true") + + results := []TaskResult{ + {TaskID: "ok", ExitCode: 0, Coverage: "92%", CoverageNum: 92, CoverageTarget: 90, KeyOutput: "done"}, + {TaskID: "warn", ExitCode: 0, Coverage: "80%", CoverageNum: 80, CoverageTarget: 90, KeyOutput: "did"}, + {TaskID: "bad", ExitCode: 2, Error: "boom"}, + } + out := generateFinalOutput(results) + + for _, sym := range []string{"PASS", "WARN", "FAIL"} { + if !strings.Contains(out, sym) { + t.Fatalf("ASCII mode should include %q, got: %s", sym, out) + } + } + for _, sym := range []string{"✓", "⚠️", "✗"} { + if strings.Contains(out, sym) { + t.Fatalf("ASCII mode should not include %q, got: %s", sym, out) + } + } + }) + + t.Run("generateFinalOutputUnicodeMode", func(t *testing.T) { + t.Setenv("CODEAGENT_ASCII_MODE", "false") + + results := []TaskResult{ + {TaskID: "ok", ExitCode: 0, Coverage: "92%", CoverageNum: 92, CoverageTarget: 90, KeyOutput: "done"}, + {TaskID: "warn", ExitCode: 0, Coverage: "80%", CoverageNum: 80, CoverageTarget: 90, KeyOutput: "did"}, + {TaskID: "bad", ExitCode: 2, Error: "boom"}, + } + out := generateFinalOutput(results) + + for _, sym := range []string{"✓", "⚠️", "✗"} { + if !strings.Contains(out, sym) { + t.Fatalf("Unicode mode should include %q, got: %s", sym, out) + } + } + }) + t.Run("executeConcurrentWrapper", func(t *testing.T) { orig := runCodexTaskFn defer func() { runCodexTaskFn = orig }() @@ -284,6 +360,18 @@ func TestExecutorRunCodexTaskWithContext(t *testing.T) { origRunner := newCommandRunner defer func() { newCommandRunner = origRunner }() + t.Run("resumeMissingSessionID", func(t *testing.T) { + newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { + t.Fatalf("unexpected command execution for invalid resume config") + return nil + } + + res := runCodexTaskWithContext(context.Background(), TaskSpec{Task: "payload", WorkDir: ".", Mode: "resume"}, nil, nil, false, false, 1) + if res.ExitCode == 0 || !strings.Contains(res.Error, "session_id") { + t.Fatalf("expected validation error, got %+v", res) + } + }) + t.Run("success", func(t *testing.T) { var firstStdout *reasonReadCloser newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { @@ -421,6 +509,100 @@ func TestExecutorRunCodexTaskWithContext(t *testing.T) { _ = closeLogger() }) + t.Run("injectedLogger", func(t *testing.T) { + newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { + return &execFakeRunner{ + stdout: newReasonReadCloser(`{"type":"item.completed","item":{"type":"agent_message","text":"injected"}}`), + process: &execFakeProcess{pid: 12}, + } + } + _ = closeLogger() + + injected, err := NewLoggerWithSuffix("executor-injected") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + defer func() { + _ = injected.Close() + _ = os.Remove(injected.Path()) + }() + + ctx := withTaskLogger(context.Background(), injected) + res := runCodexTaskWithContext(ctx, TaskSpec{ID: "task-injected", Task: "payload", WorkDir: "."}, nil, nil, false, true, 1) + if res.ExitCode != 0 || res.LogPath != injected.Path() { + t.Fatalf("expected injected logger path, got %+v", res) + } + if activeLogger() != nil { + t.Fatalf("expected no global logger to be created when injected") + } + + injected.Flush() + data, err := os.ReadFile(injected.Path()) + if err != nil { + t.Fatalf("failed to read injected log file: %v", err) + } + if !strings.Contains(string(data), "task-injected") { + t.Fatalf("injected log missing task prefix, content: %s", string(data)) + } + }) + + t.Run("contextLoggerWithoutParent", func(t *testing.T) { + newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { + return &execFakeRunner{ + stdout: newReasonReadCloser(`{"type":"item.completed","item":{"type":"agent_message","text":"ctx"}}`), + process: &execFakeProcess{pid: 14}, + } + } + _ = closeLogger() + + taskLogger, err := NewLoggerWithSuffix("executor-taskctx") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + t.Cleanup(func() { + _ = taskLogger.Close() + _ = os.Remove(taskLogger.Path()) + }) + + ctx := withTaskLogger(context.Background(), taskLogger) + res := runCodexTaskWithContext(nil, TaskSpec{ID: "task-context", Task: "payload", WorkDir: ".", Context: ctx}, nil, nil, false, true, 1) + if res.ExitCode != 0 || res.LogPath != taskLogger.Path() { + t.Fatalf("expected task logger to be reused from spec context, got %+v", res) + } + if activeLogger() != nil { + t.Fatalf("expected no global logger to be created when task context provides one") + } + + taskLogger.Flush() + data, err := os.ReadFile(taskLogger.Path()) + if err != nil { + t.Fatalf("failed to read task log: %v", err) + } + if !strings.Contains(string(data), "task-context") { + t.Fatalf("task log missing task id, content: %s", string(data)) + } + }) + + t.Run("backendSetsDirAndNilContext", func(t *testing.T) { + var rc *execFakeRunner + newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { + rc = &execFakeRunner{ + stdout: newReasonReadCloser(`{"type":"item.completed","item":{"type":"agent_message","text":"backend"}}`), + process: &execFakeProcess{pid: 13}, + } + return rc + } + + _ = closeLogger() + res := runCodexTaskWithContext(nil, TaskSpec{ID: "task-backend", Task: "payload", WorkDir: "/tmp"}, ClaudeBackend{}, nil, false, false, 1) + if res.ExitCode != 0 || res.Message != "backend" { + t.Fatalf("unexpected result: %+v", res) + } + if rc == nil || rc.dir != "/tmp" { + t.Fatalf("expected backend to set cmd.Dir, got runner=%v dir=%q", rc, rc.dir) + } + }) + t.Run("missingMessage", func(t *testing.T) { newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { return &execFakeRunner{ @@ -435,6 +617,614 @@ func TestExecutorRunCodexTaskWithContext(t *testing.T) { }) } +func TestExecutorParallelLogIsolation(t *testing.T) { + mainLogger, err := NewLoggerWithSuffix("executor-main") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + setLogger(mainLogger) + t.Cleanup(func() { + _ = closeLogger() + _ = os.Remove(mainLogger.Path()) + }) + + taskA := nextExecutorTestTaskID("iso-a") + taskB := nextExecutorTestTaskID("iso-b") + markerA := "ISOLATION_MARKER:" + taskA + markerB := "ISOLATION_MARKER:" + taskB + + origRun := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + logger := taskLoggerFromContext(task.Context) + if logger == nil { + return TaskResult{TaskID: task.ID, ExitCode: 1, Error: "missing task logger"} + } + switch task.ID { + case taskA: + logger.Info(markerA) + case taskB: + logger.Info(markerB) + default: + logger.Info("unexpected task: " + task.ID) + } + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = origRun }) + + stderrR, stderrW, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe() error = %v", err) + } + oldStderr := os.Stderr + os.Stderr = stderrW + defer func() { os.Stderr = oldStderr }() + + results := executeConcurrentWithContext(nil, [][]TaskSpec{{{ID: taskA}, {ID: taskB}}}, 1, -1) + + _ = stderrW.Close() + os.Stderr = oldStderr + stderrData, _ := io.ReadAll(stderrR) + _ = stderrR.Close() + stderrOut := string(stderrData) + + if len(results) != 2 { + t.Fatalf("expected 2 results, got %d", len(results)) + } + + paths := map[string]string{} + for _, res := range results { + if res.ExitCode != 0 { + t.Fatalf("unexpected failure: %+v", res) + } + if res.LogPath == "" { + t.Fatalf("missing LogPath for task %q", res.TaskID) + } + paths[res.TaskID] = res.LogPath + } + if paths[taskA] == paths[taskB] { + t.Fatalf("expected distinct task log paths, got %q", paths[taskA]) + } + + if strings.Contains(stderrOut, mainLogger.Path()) { + t.Fatalf("stderr should not print main log path: %s", stderrOut) + } + if !strings.Contains(stderrOut, paths[taskA]) || !strings.Contains(stderrOut, paths[taskB]) { + t.Fatalf("stderr should include task log paths, got: %s", stderrOut) + } + + mainLogger.Flush() + mainData, err := os.ReadFile(mainLogger.Path()) + if err != nil { + t.Fatalf("failed to read main log: %v", err) + } + if strings.Contains(string(mainData), markerA) || strings.Contains(string(mainData), markerB) { + t.Fatalf("main log should not contain task markers, content: %s", string(mainData)) + } + + taskAData, err := os.ReadFile(paths[taskA]) + if err != nil { + t.Fatalf("failed to read task A log: %v", err) + } + taskBData, err := os.ReadFile(paths[taskB]) + if err != nil { + t.Fatalf("failed to read task B log: %v", err) + } + if !strings.Contains(string(taskAData), markerA) || strings.Contains(string(taskAData), markerB) { + t.Fatalf("task A log isolation failed, content: %s", string(taskAData)) + } + if !strings.Contains(string(taskBData), markerB) || strings.Contains(string(taskBData), markerA) { + t.Fatalf("task B log isolation failed, content: %s", string(taskBData)) + } + + _ = os.Remove(paths[taskA]) + _ = os.Remove(paths[taskB]) +} + +func TestConcurrentExecutorParallelLogIsolationAndClosure(t *testing.T) { + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + + oldArgs := os.Args + os.Args = []string{defaultWrapperName} + t.Cleanup(func() { os.Args = oldArgs }) + + mainLogger, err := NewLoggerWithSuffix("concurrent-main") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + setLogger(mainLogger) + t.Cleanup(func() { + mainLogger.Flush() + _ = closeLogger() + _ = os.Remove(mainLogger.Path()) + }) + + const taskCount = 16 + const writersPerTask = 4 + const logsPerWriter = 50 + const expectedTaskLines = writersPerTask * logsPerWriter + + taskIDs := make([]string, 0, taskCount) + tasks := make([]TaskSpec, 0, taskCount) + for i := 0; i < taskCount; i++ { + id := nextExecutorTestTaskID("iso") + taskIDs = append(taskIDs, id) + tasks = append(tasks, TaskSpec{ID: id}) + } + + type taskLoggerInfo struct { + taskID string + logger *Logger + } + loggerCh := make(chan taskLoggerInfo, taskCount) + readyCh := make(chan struct{}, taskCount) + startCh := make(chan struct{}) + + go func() { + for i := 0; i < taskCount; i++ { + <-readyCh + } + close(startCh) + }() + + origRun := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + readyCh <- struct{}{} + + logger := taskLoggerFromContext(task.Context) + loggerCh <- taskLoggerInfo{taskID: task.ID, logger: logger} + if logger == nil { + return TaskResult{TaskID: task.ID, ExitCode: 1, Error: "missing task logger"} + } + + <-startCh + + var wg sync.WaitGroup + wg.Add(writersPerTask) + for g := 0; g < writersPerTask; g++ { + go func(g int) { + defer wg.Done() + for i := 0; i < logsPerWriter; i++ { + logger.Info(fmt.Sprintf("TASK=%s g=%d i=%d", task.ID, g, i)) + } + }(g) + } + wg.Wait() + + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = origRun }) + + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{tasks}, 1, 0) + + if len(results) != taskCount { + t.Fatalf("expected %d results, got %d", taskCount, len(results)) + } + + taskLogPaths := make(map[string]string, taskCount) + seenPaths := make(map[string]struct{}, taskCount) + for _, res := range results { + if res.ExitCode != 0 || res.Error != "" { + t.Fatalf("unexpected task failure: %+v", res) + } + if res.LogPath == "" { + t.Fatalf("missing LogPath for task %q", res.TaskID) + } + if _, ok := taskLogPaths[res.TaskID]; ok { + t.Fatalf("duplicate TaskID in results: %q", res.TaskID) + } + taskLogPaths[res.TaskID] = res.LogPath + if _, ok := seenPaths[res.LogPath]; ok { + t.Fatalf("expected unique log path per task; duplicate path %q", res.LogPath) + } + seenPaths[res.LogPath] = struct{}{} + } + if len(taskLogPaths) != taskCount { + t.Fatalf("expected %d unique task IDs, got %d", taskCount, len(taskLogPaths)) + } + + prefix := primaryLogPrefix() + pid := os.Getpid() + for _, id := range taskIDs { + path := taskLogPaths[id] + if path == "" { + t.Fatalf("missing log path for task %q", id) + } + if _, err := os.Stat(path); err != nil { + t.Fatalf("task log file not created for %q: %v", id, err) + } + wantBase := fmt.Sprintf("%s-%d-%s.log", prefix, pid, id) + if got := filepath.Base(path); got != wantBase { + t.Fatalf("unexpected log filename for %q: got %q, want %q", id, got, wantBase) + } + } + + loggers := make(map[string]*Logger, taskCount) + for i := 0; i < taskCount; i++ { + info := <-loggerCh + if info.taskID == "" { + t.Fatalf("missing taskID in logger info") + } + if info.logger == nil { + t.Fatalf("missing logger in context for task %q", info.taskID) + } + if prev, ok := loggers[info.taskID]; ok && prev != info.logger { + t.Fatalf("task %q received multiple logger instances", info.taskID) + } + loggers[info.taskID] = info.logger + } + if len(loggers) != taskCount { + t.Fatalf("expected %d task loggers, got %d", taskCount, len(loggers)) + } + + for taskID, logger := range loggers { + if !logger.closed.Load() { + t.Fatalf("expected task logger to be closed for %q", taskID) + } + if logger.file == nil { + t.Fatalf("expected task logger file to be non-nil for %q", taskID) + } + if _, err := logger.file.Write([]byte("x")); err == nil { + t.Fatalf("expected task logger file to be closed for %q", taskID) + } + } + + mainLogger.Flush() + mainData, err := os.ReadFile(mainLogger.Path()) + if err != nil { + t.Fatalf("failed to read main log: %v", err) + } + mainText := string(mainData) + if !strings.Contains(mainText, "parallel: worker_limit=") { + t.Fatalf("expected main log to include concurrency planning, content: %s", mainText) + } + if strings.Contains(mainText, "TASK=") { + t.Fatalf("main log should not contain task output, content: %s", mainText) + } + + for taskID, path := range taskLogPaths { + f, err := os.Open(path) + if err != nil { + t.Fatalf("failed to open task log for %q: %v", taskID, err) + } + + scanner := bufio.NewScanner(f) + lines := 0 + for scanner.Scan() { + line := scanner.Text() + if strings.Contains(line, "parallel:") { + t.Fatalf("task log should not contain main log entries for %q: %s", taskID, line) + } + gotID, ok := parseTaskIDFromLogLine(line) + if !ok { + t.Fatalf("task log entry missing task marker for %q: %s", taskID, line) + } + if gotID != taskID { + t.Fatalf("task log isolation failed: file=%q got TASK=%q want TASK=%q", path, gotID, taskID) + } + lines++ + } + if err := scanner.Err(); err != nil { + _ = f.Close() + t.Fatalf("scanner error for %q: %v", taskID, err) + } + if err := f.Close(); err != nil { + t.Fatalf("failed to close task log for %q: %v", taskID, err) + } + if lines != expectedTaskLines { + t.Fatalf("unexpected task log line count for %q: got %d, want %d", taskID, lines, expectedTaskLines) + } + } + + for _, path := range taskLogPaths { + _ = os.Remove(path) + } +} + +func parseTaskIDFromLogLine(line string) (string, bool) { + const marker = "TASK=" + idx := strings.Index(line, marker) + if idx == -1 { + return "", false + } + rest := line[idx+len(marker):] + end := strings.IndexByte(rest, ' ') + if end == -1 { + return rest, rest != "" + } + return rest[:end], rest[:end] != "" +} + +func TestExecutorTaskLoggerContext(t *testing.T) { + if taskLoggerFromContext(nil) != nil { + t.Fatalf("expected nil logger from nil context") + } + if taskLoggerFromContext(context.Background()) != nil { + t.Fatalf("expected nil logger when context has no logger") + } + + logger, err := NewLoggerWithSuffix("executor-taskctx") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + defer func() { + _ = logger.Close() + _ = os.Remove(logger.Path()) + }() + + ctx := withTaskLogger(context.Background(), logger) + if got := taskLoggerFromContext(ctx); got != logger { + t.Fatalf("expected logger roundtrip, got %v", got) + } + + if taskLoggerFromContext(withTaskLogger(context.Background(), nil)) != nil { + t.Fatalf("expected nil logger when injected logger is nil") + } +} + +func TestExecutorExecuteConcurrentWithContextBranches(t *testing.T) { + devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatalf("failed to open %s: %v", os.DevNull, err) + } + oldStderr := os.Stderr + os.Stderr = devNull + t.Cleanup(func() { + os.Stderr = oldStderr + _ = devNull.Close() + }) + + t.Run("skipOnFailedDependencies", func(t *testing.T) { + root := nextExecutorTestTaskID("root") + child := nextExecutorTestTaskID("child") + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + if task.ID == root { + return TaskResult{TaskID: task.ID, ExitCode: 1, Error: "boom"} + } + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{ + {{ID: root}}, + {{ID: child, Dependencies: []string{root}}}, + }, 1, 0) + + foundChild := false + for _, res := range results { + if res.LogPath != "" { + _ = os.Remove(res.LogPath) + } + if res.TaskID != child { + continue + } + foundChild = true + if res.ExitCode == 0 || !strings.Contains(res.Error, "skipped") { + t.Fatalf("expected skipped child task result, got %+v", res) + } + } + if !foundChild { + t.Fatalf("expected child task to be present in results") + } + }) + + t.Run("panicRecovered", func(t *testing.T) { + taskID := nextExecutorTestTaskID("panic") + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + panic("boom") + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{{{ID: taskID}}}, 1, 0) + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + if results[0].ExitCode == 0 || !strings.Contains(results[0].Error, "panic") { + t.Fatalf("expected panic result, got %+v", results[0]) + } + if results[0].LogPath == "" { + t.Fatalf("expected LogPath on panic result") + } + _ = os.Remove(results[0].LogPath) + }) + + t.Run("cancelWhileWaitingForWorker", func(t *testing.T) { + task1 := nextExecutorTestTaskID("slot") + task2 := nextExecutorTestTaskID("slot") + + parentCtx, cancel := context.WithCancel(context.Background()) + started := make(chan struct{}) + unblock := make(chan struct{}) + var startedOnce sync.Once + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + startedOnce.Do(func() { close(started) }) + <-unblock + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + go func() { + <-started + cancel() + time.Sleep(50 * time.Millisecond) + close(unblock) + }() + + results := executeConcurrentWithContext(parentCtx, [][]TaskSpec{{{ID: task1}, {ID: task2}}}, 1, 1) + foundCancelled := false + for _, res := range results { + if res.LogPath != "" { + _ = os.Remove(res.LogPath) + } + if res.ExitCode == 130 { + foundCancelled = true + } + } + if !foundCancelled { + t.Fatalf("expected a task to be cancelled") + } + }) + + t.Run("loggerCreateFails", func(t *testing.T) { + taskID := nextExecutorTestTaskID("bad") + "/id" + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{{{ID: taskID}}}, 1, 0) + if len(results) != 1 || results[0].ExitCode != 0 { + t.Fatalf("unexpected results: %+v", results) + } + }) + + t.Run("TestConcurrentTaskLoggerFailure", func(t *testing.T) { + // Create a writable temp dir for the main logger, then flip TMPDIR to a read-only + // location so task-specific loggers fail to open. + writable := t.TempDir() + t.Setenv("TMPDIR", writable) + + mainLogger, err := NewLoggerWithSuffix("shared-main") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + setLogger(mainLogger) + t.Cleanup(func() { + mainLogger.Flush() + _ = closeLogger() + _ = os.Remove(mainLogger.Path()) + }) + + noWrite := filepath.Join(writable, "ro") + if err := os.Mkdir(noWrite, 0o500); err != nil { + t.Fatalf("failed to create read-only temp dir: %v", err) + } + t.Setenv("TMPDIR", noWrite) + + taskA := nextExecutorTestTaskID("shared-a") + taskB := nextExecutorTestTaskID("shared-b") + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + logger := taskLoggerFromContext(task.Context) + if logger != mainLogger { + return TaskResult{TaskID: task.ID, ExitCode: 1, Error: "unexpected logger"} + } + logger.Info("TASK=" + task.ID) + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + stderrR, stderrW, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe() error = %v", err) + } + oldStderr := os.Stderr + os.Stderr = stderrW + + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{{{ID: taskA}, {ID: taskB}}}, 1, 0) + + _ = stderrW.Close() + os.Stderr = oldStderr + stderrData, _ := io.ReadAll(stderrR) + _ = stderrR.Close() + stderrOut := string(stderrData) + + if len(results) != 2 { + t.Fatalf("expected 2 results, got %d", len(results)) + } + for _, res := range results { + if res.ExitCode != 0 || res.Error != "" { + t.Fatalf("task failed unexpectedly: %+v", res) + } + if res.LogPath != mainLogger.Path() { + t.Fatalf("shared log path mismatch: got %q want %q", res.LogPath, mainLogger.Path()) + } + if !res.sharedLog { + t.Fatalf("expected sharedLog flag for %+v", res) + } + if !strings.Contains(stderrOut, "Log (shared)") { + t.Fatalf("stderr missing shared marker: %s", stderrOut) + } + } + + // Test full output mode for shared marker (summary mode doesn't show it) + summary := generateFinalOutputWithMode(results, false) + if !strings.Contains(summary, "(shared)") { + t.Fatalf("full output missing shared marker: %s", summary) + } + + mainLogger.Flush() + data, err := os.ReadFile(mainLogger.Path()) + if err != nil { + t.Fatalf("failed to read main log: %v", err) + } + content := string(data) + if !strings.Contains(content, "TASK="+taskA) || !strings.Contains(content, "TASK="+taskB) { + t.Fatalf("expected shared log to contain both tasks, got: %s", content) + } + }) + + t.Run("TestSanitizeTaskID", func(t *testing.T) { + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + logger := taskLoggerFromContext(task.Context) + if logger == nil { + return TaskResult{TaskID: task.ID, ExitCode: 1, Error: "missing logger"} + } + logger.Info("TASK=" + task.ID) + return TaskResult{TaskID: task.ID, ExitCode: 0} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + idA := "../bad id" + idB := "tab\tid" + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{{{ID: idA}, {ID: idB}}}, 1, 0) + + if len(results) != 2 { + t.Fatalf("expected 2 results, got %d", len(results)) + } + + expected := map[string]string{ + idA: sanitizeLogSuffix(idA), + idB: sanitizeLogSuffix(idB), + } + + for _, res := range results { + if res.ExitCode != 0 || res.Error != "" { + t.Fatalf("unexpected failure: %+v", res) + } + safe, ok := expected[res.TaskID] + if !ok { + t.Fatalf("unexpected task id %q in results", res.TaskID) + } + wantBase := fmt.Sprintf("%s-%d-%s.log", primaryLogPrefix(), os.Getpid(), safe) + if filepath.Base(res.LogPath) != wantBase { + t.Fatalf("log filename for %q = %q, want %q", res.TaskID, filepath.Base(res.LogPath), wantBase) + } + data, err := os.ReadFile(res.LogPath) + if err != nil { + t.Fatalf("failed to read log %q: %v", res.LogPath, err) + } + if !strings.Contains(string(data), "TASK="+res.TaskID) { + t.Fatalf("log for %q missing task marker, content: %s", res.TaskID, string(data)) + } + _ = os.Remove(res.LogPath) + } + }) +} + func TestExecutorSignalAndTermination(t *testing.T) { forceKillDelay.Store(0) defer forceKillDelay.Store(5) @@ -575,3 +1365,70 @@ func TestExecutorForwardSignalsDefaults(t *testing.T) { forwardSignals(ctx, &execFakeRunner{process: &execFakeProcess{pid: 80}}, func(string) {}) time.Sleep(10 * time.Millisecond) } + +func TestExecutorSharedLogFalseWhenCustomLogPath(t *testing.T) { + devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatalf("failed to open %s: %v", os.DevNull, err) + } + oldStderr := os.Stderr + os.Stderr = devNull + t.Cleanup(func() { + os.Stderr = oldStderr + _ = devNull.Close() + }) + + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + + // Setup: 创建主 logger + mainLogger, err := NewLoggerWithSuffix("shared-main") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + setLogger(mainLogger) + defer func() { + _ = closeLogger() + _ = os.Remove(mainLogger.Path()) + }() + + // 模拟场景：task logger 创建失败（通过设置只读的 TMPDIR）， + // 回退到主 logger（handle.shared=true）， + // 但 runCodexTaskFn 返回自定义的 LogPath（不等于主 logger 的路径） + roDir := filepath.Join(tempDir, "ro") + if err := os.Mkdir(roDir, 0o500); err != nil { + t.Fatalf("failed to create read-only dir: %v", err) + } + t.Setenv("TMPDIR", roDir) + + orig := runCodexTaskFn + customLogPath := "/custom/path/to.log" + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + // 返回自定义 LogPath，不等于主 logger 的路径 + return TaskResult{ + TaskID: task.ID, + ExitCode: 0, + LogPath: customLogPath, + } + } + defer func() { runCodexTaskFn = orig }() + + // 执行任务 + results := executeConcurrentWithContext(context.Background(), [][]TaskSpec{{{ID: "task1"}}}, 1, 0) + + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + + res := results[0] + // 关键断言：即使 handle.shared=true（因为 task logger 创建失败）， + // 但因为 LogPath 不等于主 logger 的路径，sharedLog 应为 false + if res.sharedLog { + t.Fatalf("expected sharedLog=false when LogPath differs from shared logger, got true") + } + + // 验证 LogPath 确实是自定义的 + if res.LogPath != customLogPath { + t.Fatalf("expected custom LogPath %s, got %s", customLogPath, res.LogPath) + } +} diff --git a/codeagent-wrapper/filter.go b/codeagent-wrapper/filter.go new file mode 100644 index 0000000..ab5c522 --- /dev/null +++ b/codeagent-wrapper/filter.go @@ -0,0 +1,66 @@ +package main + +import ( + "bytes" + "io" + "strings" +) + +// geminiNoisePatterns contains stderr patterns to filter for gemini backend +var geminiNoisePatterns = []string{ + "[STARTUP]", + "Session cleanup disabled", + "Warning:", + "(node:", + "(Use `node --trace-warnings", + "Loaded cached credentials", + "Loading extension:", + "YOLO mode is enabled", +} + +// filteringWriter wraps an io.Writer and filters out lines matching patterns +type filteringWriter struct { + w io.Writer + patterns []string + buf bytes.Buffer +} + +func newFilteringWriter(w io.Writer, patterns []string) *filteringWriter { + return &filteringWriter{w: w, patterns: patterns} +} + +func (f *filteringWriter) Write(p []byte) (n int, err error) { + f.buf.Write(p) + for { + line, err := f.buf.ReadString('\n') + if err != nil { + // incomplete line, put it back + f.buf.WriteString(line) + break + } + if !f.shouldFilter(line) { + f.w.Write([]byte(line)) + } + } + return len(p), nil +} + +func (f *filteringWriter) shouldFilter(line string) bool { + for _, pattern := range f.patterns { + if strings.Contains(line, pattern) { + return true + } + } + return false +} + +// Flush writes any remaining buffered content +func (f *filteringWriter) Flush() { + if f.buf.Len() > 0 { + remaining := f.buf.String() + if !f.shouldFilter(remaining) { + f.w.Write([]byte(remaining)) + } + f.buf.Reset() + } +} diff --git a/codeagent-wrapper/filter_test.go b/codeagent-wrapper/filter_test.go new file mode 100644 index 0000000..12042f8 --- /dev/null +++ b/codeagent-wrapper/filter_test.go @@ -0,0 +1,73 @@ +package main + +import ( + "bytes" + "testing" +) + +func TestFilteringWriter(t *testing.T) { + tests := []struct { + name string + patterns []string + input string + want string + }{ + { + name: "filter STARTUP lines", + patterns: geminiNoisePatterns, + input: "[STARTUP] Recording metric\nHello World\n[STARTUP] Another line\n", + want: "Hello World\n", + }, + { + name: "filter Warning lines", + patterns: geminiNoisePatterns, + input: "Warning: something bad\nActual output\n", + want: "Actual output\n", + }, + { + name: "filter multiple patterns", + patterns: geminiNoisePatterns, + input: "YOLO mode is enabled\nSession cleanup disabled\nReal content\nLoading extension: foo\n", + want: "Real content\n", + }, + { + name: "no filtering needed", + patterns: geminiNoisePatterns, + input: "Line 1\nLine 2\nLine 3\n", + want: "Line 1\nLine 2\nLine 3\n", + }, + { + name: "empty input", + patterns: geminiNoisePatterns, + input: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + fw := newFilteringWriter(&buf, tt.patterns) + fw.Write([]byte(tt.input)) + fw.Flush() + + if got := buf.String(); got != tt.want { + t.Errorf("got %q, want %q", got, tt.want) + } + }) + } +} + +func TestFilteringWriterPartialLines(t *testing.T) { + var buf bytes.Buffer + fw := newFilteringWriter(&buf, geminiNoisePatterns) + + // Write partial line + fw.Write([]byte("Hello ")) + fw.Write([]byte("World\n")) + fw.Flush() + + if got := buf.String(); got != "Hello World\n" { + t.Errorf("got %q, want %q", got, "Hello World\n") + } +} diff --git a/codeagent-wrapper/log_writer_limit_test.go b/codeagent-wrapper/log_writer_limit_test.go new file mode 100644 index 0000000..9f51c07 --- /dev/null +++ b/codeagent-wrapper/log_writer_limit_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "os" + "strings" + "testing" +) + +func TestLogWriterWriteLimitsBuffer(t *testing.T) { + defer resetTestHooks() + + logger, err := NewLogger() + if err != nil { + t.Fatalf("NewLogger error: %v", err) + } + setLogger(logger) + defer closeLogger() + + lw := newLogWriter("P:", 10) + _, _ = lw.Write([]byte(strings.Repeat("a", 100))) + + if lw.buf.Len() != 10 { + t.Fatalf("logWriter buffer len=%d, want %d", lw.buf.Len(), 10) + } + if !lw.dropped { + t.Fatalf("expected logWriter to drop overlong line bytes") + } + + lw.Flush() + logger.Flush() + data, err := os.ReadFile(logger.Path()) + if err != nil { + t.Fatalf("ReadFile error: %v", err) + } + if !strings.Contains(string(data), "P:aaaaaaa...") { + t.Fatalf("log output missing truncated entry, got %q", string(data)) + } +} + diff --git a/codeagent-wrapper/logger.go b/codeagent-wrapper/logger.go index f33299e..425cfc4 100644 --- a/codeagent-wrapper/logger.go +++ b/codeagent-wrapper/logger.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "hash/crc32" "os" "path/filepath" "strconv" @@ -18,22 +19,25 @@ import ( // It is intentionally minimal: a buffered channel + single worker goroutine // to avoid contention while keeping ordering guarantees. type Logger struct { - path string - file *os.File - writer *bufio.Writer - ch chan logEntry - flushReq chan chan struct{} - done chan struct{} - closed atomic.Bool - closeOnce sync.Once - workerWG sync.WaitGroup - pendingWG sync.WaitGroup - flushMu sync.Mutex + path string + file *os.File + writer *bufio.Writer + ch chan logEntry + flushReq chan chan struct{} + done chan struct{} + closed atomic.Bool + closeOnce sync.Once + workerWG sync.WaitGroup + pendingWG sync.WaitGroup + flushMu sync.Mutex + workerErr error + errorEntries []string // Cache of recent ERROR/WARN entries + errorMu sync.Mutex } type logEntry struct { - level string - msg string + msg string + isError bool // true for ERROR or WARN levels } // CleanupStats captures the outcome of a cleanupOldLogs run. @@ -55,6 +59,10 @@ var ( evalSymlinksFn = filepath.EvalSymlinks ) +const maxLogSuffixLen = 64 + +var logSuffixCounter atomic.Uint64 + // NewLogger creates the async logger and starts the worker goroutine. // The log file is created under os.TempDir() using the required naming scheme. func NewLogger() (*Logger, error) { @@ -64,14 +72,23 @@ func NewLogger() (*Logger, error) { // NewLoggerWithSuffix creates a logger with an optional suffix in the filename. // Useful for tests that need isolated log files within the same process. func NewLoggerWithSuffix(suffix string) (*Logger, error) { - filename := fmt.Sprintf("%s-%d", primaryLogPrefix(), os.Getpid()) + pid := os.Getpid() + filename := fmt.Sprintf("%s-%d", primaryLogPrefix(), pid) + var safeSuffix string if suffix != "" { - filename += "-" + suffix + safeSuffix = sanitizeLogSuffix(suffix) + } + if safeSuffix != "" { + filename += "-" + safeSuffix } filename += ".log" path := filepath.Clean(filepath.Join(os.TempDir(), filename)) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return nil, err + } + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) if err != nil { return nil, err @@ -92,6 +109,73 @@ func NewLoggerWithSuffix(suffix string) (*Logger, error) { return l, nil } +func sanitizeLogSuffix(raw string) string { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return fallbackLogSuffix() + } + + var b strings.Builder + changed := false + for _, r := range trimmed { + if isSafeLogRune(r) { + b.WriteRune(r) + } else { + changed = true + b.WriteByte('-') + } + if b.Len() >= maxLogSuffixLen { + changed = true + break + } + } + + sanitized := strings.Trim(b.String(), "-.") + if sanitized != b.String() { + changed = true // Mark if trim removed any characters + } + if sanitized == "" { + return fallbackLogSuffix() + } + + if changed || len(sanitized) > maxLogSuffixLen { + hash := crc32.ChecksumIEEE([]byte(trimmed)) + hashStr := fmt.Sprintf("%x", hash) + + maxPrefix := maxLogSuffixLen - len(hashStr) - 1 + if maxPrefix < 1 { + maxPrefix = 1 + } + if len(sanitized) > maxPrefix { + sanitized = sanitized[:maxPrefix] + } + + sanitized = fmt.Sprintf("%s-%s", sanitized, hashStr) + } + + return sanitized +} + +func fallbackLogSuffix() string { + next := logSuffixCounter.Add(1) + return fmt.Sprintf("task-%d", next) +} + +func isSafeLogRune(r rune) bool { + switch { + case r >= 'a' && r <= 'z': + return true + case r >= 'A' && r <= 'Z': + return true + case r >= '0' && r <= '9': + return true + case r == '-', r == '_', r == '.': + return true + default: + return false + } +} + // Path returns the underlying log file path (useful for tests/inspection). func (l *Logger) Path() string { if l == nil { @@ -112,10 +196,11 @@ func (l *Logger) Debug(msg string) { l.log("DEBUG", msg) } // Error logs at ERROR level. func (l *Logger) Error(msg string) { l.log("ERROR", msg) } -// Close stops the worker and syncs the log file. +// Close signals the worker to flush and close the log file. // The log file is NOT removed, allowing inspection after program exit. // It is safe to call multiple times. -// Returns after a 5-second timeout if worker doesn't stop gracefully. +// Waits up to CODEAGENT_LOGGER_CLOSE_TIMEOUT_MS (default: 5000) for shutdown; set to 0 to wait indefinitely. +// Returns an error if shutdown doesn't complete within the timeout. func (l *Logger) Close() error { if l == nil { return nil @@ -126,42 +211,51 @@ func (l *Logger) Close() error { l.closeOnce.Do(func() { l.closed.Store(true) close(l.done) - close(l.ch) - // Wait for worker with timeout + timeout := loggerCloseTimeout() workerDone := make(chan struct{}) go func() { l.workerWG.Wait() close(workerDone) }() - select { - case <-workerDone: - // Worker stopped gracefully - case <-time.After(5 * time.Second): - // Worker timeout - proceed with cleanup anyway - closeErr = fmt.Errorf("logger worker timeout during close") + if timeout > 0 { + select { + case <-workerDone: + // Worker stopped gracefully + case <-time.After(timeout): + closeErr = fmt.Errorf("logger worker timeout during close") + return + } + } else { + <-workerDone } - if err := l.writer.Flush(); err != nil && closeErr == nil { - closeErr = err + if l.workerErr != nil && closeErr == nil { + closeErr = l.workerErr } - - if err := l.file.Sync(); err != nil && closeErr == nil { - closeErr = err - } - - if err := l.file.Close(); err != nil && closeErr == nil { - closeErr = err - } - - // Log file is kept for debugging - NOT removed - // Users can manually clean up /tmp/-*.log files }) return closeErr } +func loggerCloseTimeout() time.Duration { + const defaultTimeout = 5 * time.Second + + raw := strings.TrimSpace(os.Getenv("CODEAGENT_LOGGER_CLOSE_TIMEOUT_MS")) + if raw == "" { + return defaultTimeout + } + ms, err := strconv.Atoi(raw) + if err != nil { + return defaultTimeout + } + if ms <= 0 { + return 0 + } + return time.Duration(ms) * time.Millisecond +} + // RemoveLogFile removes the log file. Should only be called after Close(). func (l *Logger) RemoveLogFile() error { if l == nil { @@ -170,34 +264,29 @@ func (l *Logger) RemoveLogFile() error { return os.Remove(l.path) } -// ExtractRecentErrors reads the log file and returns the most recent ERROR and WARN entries. +// ExtractRecentErrors returns the most recent ERROR and WARN entries from memory cache. // Returns up to maxEntries entries in chronological order. func (l *Logger) ExtractRecentErrors(maxEntries int) []string { - if l == nil || l.path == "" { + if l == nil || maxEntries <= 0 { return nil } - f, err := os.Open(l.path) - if err != nil { + l.errorMu.Lock() + defer l.errorMu.Unlock() + + if len(l.errorEntries) == 0 { return nil } - defer f.Close() - var entries []string - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := scanner.Text() - if strings.Contains(line, "] ERROR:") || strings.Contains(line, "] WARN:") { - entries = append(entries, line) - } + // Return last N entries + start := 0 + if len(l.errorEntries) > maxEntries { + start = len(l.errorEntries) - maxEntries } - // Keep only the last maxEntries - if len(entries) > maxEntries { - entries = entries[len(entries)-maxEntries:] - } - - return entries + result := make([]string, len(l.errorEntries)-start) + copy(result, l.errorEntries[start:]) + return result } // Flush waits for all pending log entries to be written. Primarily for tests. @@ -254,7 +343,8 @@ func (l *Logger) log(level, msg string) { return } - entry := logEntry{level: level, msg: msg} + isError := level == "WARN" || level == "ERROR" + entry := logEntry{msg: msg, isError: isError} l.flushMu.Lock() l.pendingWG.Add(1) l.flushMu.Unlock() @@ -275,18 +365,43 @@ func (l *Logger) run() { ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() + writeEntry := func(entry logEntry) { + timestamp := time.Now().Format("2006-01-02 15:04:05.000") + fmt.Fprintf(l.writer, "[%s] %s\n", timestamp, entry.msg) + + // Cache error/warn entries in memory for fast extraction + if entry.isError { + l.errorMu.Lock() + l.errorEntries = append(l.errorEntries, entry.msg) + if len(l.errorEntries) > 100 { // Keep last 100 + l.errorEntries = l.errorEntries[1:] + } + l.errorMu.Unlock() + } + + l.pendingWG.Done() + } + + finalize := func() { + if err := l.writer.Flush(); err != nil && l.workerErr == nil { + l.workerErr = err + } + if err := l.file.Sync(); err != nil && l.workerErr == nil { + l.workerErr = err + } + if err := l.file.Close(); err != nil && l.workerErr == nil { + l.workerErr = err + } + } + for { select { case entry, ok := <-l.ch: if !ok { - // Channel closed, final flush - _ = l.writer.Flush() + finalize() return } - timestamp := time.Now().Format("2006-01-02 15:04:05.000") - pid := os.Getpid() - fmt.Fprintf(l.writer, "[%s] [PID:%d] %s: %s\n", timestamp, pid, entry.level, entry.msg) - l.pendingWG.Done() + writeEntry(entry) case <-ticker.C: _ = l.writer.Flush() @@ -296,6 +411,21 @@ func (l *Logger) run() { _ = l.writer.Flush() _ = l.file.Sync() close(flushDone) + + case <-l.done: + for { + select { + case entry, ok := <-l.ch: + if !ok { + finalize() + return + } + writeEntry(entry) + default: + finalize() + return + } + } } } } diff --git a/codeagent-wrapper/logger_additional_coverage_test.go b/codeagent-wrapper/logger_additional_coverage_test.go new file mode 100644 index 0000000..0e8be30 --- /dev/null +++ b/codeagent-wrapper/logger_additional_coverage_test.go @@ -0,0 +1,158 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestLoggerNilReceiverNoop(t *testing.T) { + var logger *Logger + logger.Info("info") + logger.Warn("warn") + logger.Debug("debug") + logger.Error("error") + logger.Flush() + if err := logger.Close(); err != nil { + t.Fatalf("Close() on nil logger should return nil, got %v", err) + } +} + +func TestLoggerConcurrencyLogHelpers(t *testing.T) { + setTempDirEnv(t, t.TempDir()) + + logger, err := NewLoggerWithSuffix("concurrency") + if err != nil { + t.Fatalf("NewLoggerWithSuffix error: %v", err) + } + setLogger(logger) + defer closeLogger() + + logConcurrencyPlanning(0, 2) + logConcurrencyPlanning(3, 2) + logConcurrencyState("start", "task-1", 1, 0) + logConcurrencyState("done", "task-1", 0, 3) + logger.Flush() + + data, err := os.ReadFile(logger.Path()) + if err != nil { + t.Fatalf("failed to read log file: %v", err) + } + output := string(data) + + checks := []string{ + "parallel: worker_limit=unbounded total_tasks=2", + "parallel: worker_limit=3 total_tasks=2", + "parallel: start task=task-1 active=1 limit=unbounded", + "parallel: done task=task-1 active=0 limit=3", + } + for _, c := range checks { + if !strings.Contains(output, c) { + t.Fatalf("log output missing %q, got: %s", c, output) + } + } +} + +func TestLoggerConcurrencyLogHelpersNoopWithoutActiveLogger(t *testing.T) { + _ = closeLogger() + logConcurrencyPlanning(1, 1) + logConcurrencyState("start", "task-1", 0, 1) +} + +func TestLoggerCleanupOldLogsSkipsUnsafeAndHandlesAlreadyDeleted(t *testing.T) { + tempDir := setTempDirEnv(t, t.TempDir()) + + unsafePath := createTempLog(t, tempDir, fmt.Sprintf("%s-%d.log", primaryLogPrefix(), 222)) + orphanPath := createTempLog(t, tempDir, fmt.Sprintf("%s-%d.log", primaryLogPrefix(), 111)) + + stubFileStat(t, func(path string) (os.FileInfo, error) { + if path == unsafePath { + return fakeFileInfo{mode: os.ModeSymlink}, nil + } + return os.Lstat(path) + }) + + stubProcessRunning(t, func(pid int) bool { + if pid == 111 { + _ = os.Remove(orphanPath) + } + return false + }) + + stats, err := cleanupOldLogs() + if err != nil { + t.Fatalf("cleanupOldLogs() unexpected error: %v", err) + } + + if stats.Scanned != 2 { + t.Fatalf("scanned = %d, want %d", stats.Scanned, 2) + } + if stats.Deleted != 0 { + t.Fatalf("deleted = %d, want %d", stats.Deleted, 0) + } + if stats.Kept != 2 { + t.Fatalf("kept = %d, want %d", stats.Kept, 2) + } + if stats.Errors != 0 { + t.Fatalf("errors = %d, want %d", stats.Errors, 0) + } + + hasSkip := false + hasAlreadyDeleted := false + for _, name := range stats.KeptFiles { + if strings.Contains(name, "already deleted") { + hasAlreadyDeleted = true + } + if strings.Contains(name, filepath.Base(unsafePath)) { + hasSkip = true + } + } + if !hasSkip { + t.Fatalf("expected kept files to include unsafe log %q, got %+v", filepath.Base(unsafePath), stats.KeptFiles) + } + if !hasAlreadyDeleted { + t.Fatalf("expected kept files to include already deleted marker, got %+v", stats.KeptFiles) + } +} + +func TestLoggerIsUnsafeFileErrorPaths(t *testing.T) { + tempDir := t.TempDir() + + t.Run("stat ErrNotExist", func(t *testing.T) { + stubFileStat(t, func(string) (os.FileInfo, error) { + return nil, os.ErrNotExist + }) + + unsafe, reason := isUnsafeFile("missing.log", tempDir) + if !unsafe || reason != "" { + t.Fatalf("expected missing file to be skipped silently, got unsafe=%v reason=%q", unsafe, reason) + } + }) + + t.Run("stat error", func(t *testing.T) { + stubFileStat(t, func(string) (os.FileInfo, error) { + return nil, fmt.Errorf("boom") + }) + + unsafe, reason := isUnsafeFile("broken.log", tempDir) + if !unsafe || !strings.Contains(reason, "stat failed") { + t.Fatalf("expected stat failure to be unsafe, got unsafe=%v reason=%q", unsafe, reason) + } + }) + + t.Run("EvalSymlinks error", func(t *testing.T) { + stubFileStat(t, func(string) (os.FileInfo, error) { + return fakeFileInfo{}, nil + }) + stubEvalSymlinks(t, func(string) (string, error) { + return "", fmt.Errorf("resolve failed") + }) + + unsafe, reason := isUnsafeFile("cannot-resolve.log", tempDir) + if !unsafe || !strings.Contains(reason, "path resolution failed") { + t.Fatalf("expected resolution failure to be unsafe, got unsafe=%v reason=%q", unsafe, reason) + } + }) +} diff --git a/codeagent-wrapper/logger_suffix_test.go b/codeagent-wrapper/logger_suffix_test.go new file mode 100644 index 0000000..dc4a94f --- /dev/null +++ b/codeagent-wrapper/logger_suffix_test.go @@ -0,0 +1,115 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestLoggerWithSuffixNamingAndIsolation(t *testing.T) { + tempDir := setTempDirEnv(t, t.TempDir()) + + taskA := "task-1" + taskB := "task-2" + + loggerA, err := NewLoggerWithSuffix(taskA) + if err != nil { + t.Fatalf("NewLoggerWithSuffix(%q) error = %v", taskA, err) + } + defer loggerA.Close() + + loggerB, err := NewLoggerWithSuffix(taskB) + if err != nil { + t.Fatalf("NewLoggerWithSuffix(%q) error = %v", taskB, err) + } + defer loggerB.Close() + + wantA := filepath.Join(tempDir, fmt.Sprintf("%s-%d-%s.log", primaryLogPrefix(), os.Getpid(), taskA)) + if loggerA.Path() != wantA { + t.Fatalf("loggerA path = %q, want %q", loggerA.Path(), wantA) + } + + wantB := filepath.Join(tempDir, fmt.Sprintf("%s-%d-%s.log", primaryLogPrefix(), os.Getpid(), taskB)) + if loggerB.Path() != wantB { + t.Fatalf("loggerB path = %q, want %q", loggerB.Path(), wantB) + } + + if loggerA.Path() == loggerB.Path() { + t.Fatalf("expected different log files, got %q", loggerA.Path()) + } + + loggerA.Info("from taskA") + loggerB.Info("from taskB") + loggerA.Flush() + loggerB.Flush() + + dataA, err := os.ReadFile(loggerA.Path()) + if err != nil { + t.Fatalf("failed to read loggerA file: %v", err) + } + dataB, err := os.ReadFile(loggerB.Path()) + if err != nil { + t.Fatalf("failed to read loggerB file: %v", err) + } + + if !strings.Contains(string(dataA), "from taskA") { + t.Fatalf("loggerA missing its message, got: %q", string(dataA)) + } + if strings.Contains(string(dataA), "from taskB") { + t.Fatalf("loggerA contains loggerB message, got: %q", string(dataA)) + } + if !strings.Contains(string(dataB), "from taskB") { + t.Fatalf("loggerB missing its message, got: %q", string(dataB)) + } + if strings.Contains(string(dataB), "from taskA") { + t.Fatalf("loggerB contains loggerA message, got: %q", string(dataB)) + } +} + +func TestLoggerWithSuffixReturnsErrorWhenTempDirNotWritable(t *testing.T) { + base := t.TempDir() + noWrite := filepath.Join(base, "ro") + if err := os.Mkdir(noWrite, 0o500); err != nil { + t.Fatalf("failed to create read-only temp dir: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(noWrite, 0o700) }) + setTempDirEnv(t, noWrite) + + logger, err := NewLoggerWithSuffix("task-err") + if err == nil { + _ = logger.Close() + t.Fatalf("expected error when temp dir is not writable") + } +} + +func TestLoggerWithSuffixSanitizesUnsafeSuffix(t *testing.T) { + tempDir := setTempDirEnv(t, t.TempDir()) + + raw := "../bad id/with?chars" + safe := sanitizeLogSuffix(raw) + if safe == "" { + t.Fatalf("sanitizeLogSuffix returned empty string") + } + if strings.ContainsAny(safe, "/\\") { + t.Fatalf("sanitized suffix should not contain path separators, got %q", safe) + } + + logger, err := NewLoggerWithSuffix(raw) + if err != nil { + t.Fatalf("NewLoggerWithSuffix(%q) error = %v", raw, err) + } + t.Cleanup(func() { + _ = logger.Close() + _ = os.Remove(logger.Path()) + }) + + wantBase := fmt.Sprintf("%s-%d-%s.log", primaryLogPrefix(), os.Getpid(), safe) + if gotBase := filepath.Base(logger.Path()); gotBase != wantBase { + t.Fatalf("log filename = %q, want %q", gotBase, wantBase) + } + if dir := filepath.Dir(logger.Path()); dir != tempDir { + t.Fatalf("logger path dir = %q, want %q", dir, tempDir) + } +} diff --git a/codeagent-wrapper/logger_test.go b/codeagent-wrapper/logger_test.go index 6a9c37b..e0f5e31 100644 --- a/codeagent-wrapper/logger_test.go +++ b/codeagent-wrapper/logger_test.go @@ -26,7 +26,7 @@ func compareCleanupStats(got, want CleanupStats) bool { return true } -func TestRunLoggerCreatesFileWithPID(t *testing.T) { +func TestLoggerCreatesFileWithPID(t *testing.T) { tempDir := t.TempDir() t.Setenv("TMPDIR", tempDir) @@ -46,7 +46,7 @@ func TestRunLoggerCreatesFileWithPID(t *testing.T) { } } -func TestRunLoggerWritesLevels(t *testing.T) { +func TestLoggerWritesLevels(t *testing.T) { tempDir := t.TempDir() t.Setenv("TMPDIR", tempDir) @@ -69,7 +69,7 @@ func TestRunLoggerWritesLevels(t *testing.T) { } content := string(data) - checks := []string{"INFO: info message", "WARN: warn message", "DEBUG: debug message", "ERROR: error message"} + checks := []string{"info message", "warn message", "debug message", "error message"} for _, c := range checks { if !strings.Contains(content, c) { t.Fatalf("log file missing entry %q, content: %s", c, content) @@ -77,7 +77,31 @@ func TestRunLoggerWritesLevels(t *testing.T) { } } -func TestRunLoggerCloseRemovesFileAndStopsWorker(t *testing.T) { +func TestLoggerDefaultIsTerminalCoverage(t *testing.T) { + oldStdin := os.Stdin + t.Cleanup(func() { os.Stdin = oldStdin }) + + f, err := os.CreateTemp(t.TempDir(), "stdin-*") + if err != nil { + t.Fatalf("os.CreateTemp() error = %v", err) + } + defer os.Remove(f.Name()) + + os.Stdin = f + if got := defaultIsTerminal(); got { + t.Fatalf("defaultIsTerminal() = %v, want false for regular file", got) + } + + if err := f.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + os.Stdin = f + if got := defaultIsTerminal(); !got { + t.Fatalf("defaultIsTerminal() = %v, want true when Stat fails", got) + } +} + +func TestLoggerCloseStopsWorkerAndKeepsFile(t *testing.T) { tempDir := t.TempDir() t.Setenv("TMPDIR", tempDir) @@ -94,6 +118,11 @@ func TestRunLoggerCloseRemovesFileAndStopsWorker(t *testing.T) { if err := logger.Close(); err != nil { t.Fatalf("Close() returned error: %v", err) } + if logger.file != nil { + if _, err := logger.file.Write([]byte("x")); err == nil { + t.Fatalf("expected file to be closed after Close()") + } + } // After recent changes, log file is kept for debugging - NOT removed if _, err := os.Stat(logPath); os.IsNotExist(err) { @@ -116,7 +145,7 @@ func TestRunLoggerCloseRemovesFileAndStopsWorker(t *testing.T) { } } -func TestRunLoggerConcurrentWritesSafe(t *testing.T) { +func TestLoggerConcurrentWritesSafe(t *testing.T) { tempDir := t.TempDir() t.Setenv("TMPDIR", tempDir) @@ -165,7 +194,7 @@ func TestRunLoggerConcurrentWritesSafe(t *testing.T) { } } -func TestRunLoggerTerminateProcessActive(t *testing.T) { +func TestLoggerTerminateProcessActive(t *testing.T) { cmd := exec.Command("sleep", "5") if err := cmd.Start(); err != nil { t.Skipf("cannot start sleep command: %v", err) @@ -193,7 +222,7 @@ func TestRunLoggerTerminateProcessActive(t *testing.T) { time.Sleep(10 * time.Millisecond) } -func TestRunTerminateProcessNil(t *testing.T) { +func TestLoggerTerminateProcessNil(t *testing.T) { if timer := terminateProcess(nil); timer != nil { t.Fatalf("terminateProcess(nil) should return nil timer") } @@ -202,7 +231,7 @@ func TestRunTerminateProcessNil(t *testing.T) { } } -func TestRunCleanupOldLogsRemovesOrphans(t *testing.T) { +func TestLoggerCleanupOldLogsRemovesOrphans(t *testing.T) { tempDir := setTempDirEnv(t, t.TempDir()) orphan1 := createTempLog(t, tempDir, "codex-wrapper-111.log") @@ -252,7 +281,7 @@ func TestRunCleanupOldLogsRemovesOrphans(t *testing.T) { } } -func TestRunCleanupOldLogsHandlesInvalidNamesAndErrors(t *testing.T) { +func TestLoggerCleanupOldLogsHandlesInvalidNamesAndErrors(t *testing.T) { tempDir := setTempDirEnv(t, t.TempDir()) invalid := []string{ @@ -310,7 +339,7 @@ func TestRunCleanupOldLogsHandlesInvalidNamesAndErrors(t *testing.T) { } } -func TestRunCleanupOldLogsHandlesGlobFailures(t *testing.T) { +func TestLoggerCleanupOldLogsHandlesGlobFailures(t *testing.T) { stubProcessRunning(t, func(pid int) bool { t.Fatalf("process check should not run when glob fails") return false @@ -336,7 +365,7 @@ func TestRunCleanupOldLogsHandlesGlobFailures(t *testing.T) { } } -func TestRunCleanupOldLogsEmptyDirectoryStats(t *testing.T) { +func TestLoggerCleanupOldLogsEmptyDirectoryStats(t *testing.T) { setTempDirEnv(t, t.TempDir()) stubProcessRunning(t, func(int) bool { @@ -356,7 +385,7 @@ func TestRunCleanupOldLogsEmptyDirectoryStats(t *testing.T) { } } -func TestRunCleanupOldLogsHandlesTempDirPermissionErrors(t *testing.T) { +func TestLoggerCleanupOldLogsHandlesTempDirPermissionErrors(t *testing.T) { tempDir := setTempDirEnv(t, t.TempDir()) paths := []string{ @@ -396,7 +425,7 @@ func TestRunCleanupOldLogsHandlesTempDirPermissionErrors(t *testing.T) { } } -func TestRunCleanupOldLogsHandlesPermissionDeniedFile(t *testing.T) { +func TestLoggerCleanupOldLogsHandlesPermissionDeniedFile(t *testing.T) { tempDir := setTempDirEnv(t, t.TempDir()) protected := createTempLog(t, tempDir, "codex-wrapper-6200.log") @@ -433,7 +462,7 @@ func TestRunCleanupOldLogsHandlesPermissionDeniedFile(t *testing.T) { } } -func TestRunCleanupOldLogsPerformanceBound(t *testing.T) { +func TestLoggerCleanupOldLogsPerformanceBound(t *testing.T) { tempDir := setTempDirEnv(t, t.TempDir()) const fileCount = 400 @@ -476,17 +505,98 @@ func TestRunCleanupOldLogsPerformanceBound(t *testing.T) { } } -func TestRunCleanupOldLogsCoverageSuite(t *testing.T) { +func TestLoggerCleanupOldLogsCoverageSuite(t *testing.T) { TestBackendParseJSONStream_CoverageSuite(t) } // Reuse the existing coverage suite so the focused TestLogger run still exercises // the rest of the codebase and keeps coverage high. -func TestRunLoggerCoverageSuite(t *testing.T) { - TestBackendParseJSONStream_CoverageSuite(t) +func TestLoggerCoverageSuite(t *testing.T) { + suite := []struct { + name string + fn func(*testing.T) + }{ + {"TestBackendParseJSONStream_CoverageSuite", TestBackendParseJSONStream_CoverageSuite}, + {"TestVersionCoverageFullRun", TestVersionCoverageFullRun}, + {"TestVersionMainWrapper", TestVersionMainWrapper}, + + {"TestExecutorHelperCoverage", TestExecutorHelperCoverage}, + {"TestExecutorRunCodexTaskWithContext", TestExecutorRunCodexTaskWithContext}, + {"TestExecutorParallelLogIsolation", TestExecutorParallelLogIsolation}, + {"TestExecutorTaskLoggerContext", TestExecutorTaskLoggerContext}, + {"TestExecutorExecuteConcurrentWithContextBranches", TestExecutorExecuteConcurrentWithContextBranches}, + {"TestExecutorSignalAndTermination", TestExecutorSignalAndTermination}, + {"TestExecutorCancelReasonAndCloseWithReason", TestExecutorCancelReasonAndCloseWithReason}, + {"TestExecutorForceKillTimerStop", TestExecutorForceKillTimerStop}, + {"TestExecutorForwardSignalsDefaults", TestExecutorForwardSignalsDefaults}, + + {"TestBackendParseArgs_NewMode", TestBackendParseArgs_NewMode}, + {"TestBackendParseArgs_ResumeMode", TestBackendParseArgs_ResumeMode}, + {"TestBackendParseArgs_BackendFlag", TestBackendParseArgs_BackendFlag}, + {"TestBackendParseArgs_SkipPermissions", TestBackendParseArgs_SkipPermissions}, + {"TestBackendParseBoolFlag", TestBackendParseBoolFlag}, + {"TestBackendEnvFlagEnabled", TestBackendEnvFlagEnabled}, + {"TestRunResolveTimeout", TestRunResolveTimeout}, + {"TestRunIsTerminal", TestRunIsTerminal}, + {"TestRunReadPipedTask", TestRunReadPipedTask}, + {"TestTailBufferWrite", TestTailBufferWrite}, + {"TestLogWriterWriteLimitsBuffer", TestLogWriterWriteLimitsBuffer}, + {"TestLogWriterLogLine", TestLogWriterLogLine}, + {"TestNewLogWriterDefaultMaxLen", TestNewLogWriterDefaultMaxLen}, + {"TestNewLogWriterDefaultLimit", TestNewLogWriterDefaultLimit}, + {"TestRunHello", TestRunHello}, + {"TestRunGreet", TestRunGreet}, + {"TestRunFarewell", TestRunFarewell}, + {"TestRunFarewellEmpty", TestRunFarewellEmpty}, + + {"TestParallelParseConfig_Success", TestParallelParseConfig_Success}, + {"TestParallelParseConfig_Backend", TestParallelParseConfig_Backend}, + {"TestParallelParseConfig_InvalidFormat", TestParallelParseConfig_InvalidFormat}, + {"TestParallelParseConfig_EmptyTasks", TestParallelParseConfig_EmptyTasks}, + {"TestParallelParseConfig_MissingID", TestParallelParseConfig_MissingID}, + {"TestParallelParseConfig_MissingTask", TestParallelParseConfig_MissingTask}, + {"TestParallelParseConfig_DuplicateID", TestParallelParseConfig_DuplicateID}, + {"TestParallelParseConfig_DelimiterFormat", TestParallelParseConfig_DelimiterFormat}, + + {"TestBackendSelectBackend", TestBackendSelectBackend}, + {"TestBackendSelectBackend_Invalid", TestBackendSelectBackend_Invalid}, + {"TestBackendSelectBackend_DefaultOnEmpty", TestBackendSelectBackend_DefaultOnEmpty}, + {"TestBackendBuildArgs_CodexBackend", TestBackendBuildArgs_CodexBackend}, + {"TestBackendBuildArgs_ClaudeBackend", TestBackendBuildArgs_ClaudeBackend}, + {"TestClaudeBackendBuildArgs_OutputValidation", TestClaudeBackendBuildArgs_OutputValidation}, + {"TestBackendBuildArgs_GeminiBackend", TestBackendBuildArgs_GeminiBackend}, + {"TestGeminiBackendBuildArgs_OutputValidation", TestGeminiBackendBuildArgs_OutputValidation}, + {"TestBackendNamesAndCommands", TestBackendNamesAndCommands}, + + {"TestBackendParseJSONStream", TestBackendParseJSONStream}, + {"TestBackendParseJSONStream_ClaudeEvents", TestBackendParseJSONStream_ClaudeEvents}, + {"TestBackendParseJSONStream_GeminiEvents", TestBackendParseJSONStream_GeminiEvents}, + {"TestBackendParseJSONStreamWithWarn_InvalidLine", TestBackendParseJSONStreamWithWarn_InvalidLine}, + {"TestBackendParseJSONStream_OnMessage", TestBackendParseJSONStream_OnMessage}, + {"TestBackendParseJSONStream_ScannerError", TestBackendParseJSONStream_ScannerError}, + {"TestBackendDiscardInvalidJSON", TestBackendDiscardInvalidJSON}, + {"TestBackendDiscardInvalidJSONBuffer", TestBackendDiscardInvalidJSONBuffer}, + + {"TestCurrentWrapperNameFallsBackToExecutable", TestCurrentWrapperNameFallsBackToExecutable}, + {"TestCurrentWrapperNameDetectsLegacyAliasSymlink", TestCurrentWrapperNameDetectsLegacyAliasSymlink}, + + {"TestIsProcessRunning", TestIsProcessRunning}, + {"TestGetProcessStartTimeReadsProcStat", TestGetProcessStartTimeReadsProcStat}, + {"TestGetProcessStartTimeInvalidData", TestGetProcessStartTimeInvalidData}, + {"TestGetBootTimeParsesBtime", TestGetBootTimeParsesBtime}, + {"TestGetBootTimeInvalidData", TestGetBootTimeInvalidData}, + + {"TestClaudeBuildArgs_ModesAndPermissions", TestClaudeBuildArgs_ModesAndPermissions}, + {"TestClaudeBuildArgs_GeminiAndCodexModes", TestClaudeBuildArgs_GeminiAndCodexModes}, + {"TestClaudeBuildArgs_BackendMetadata", TestClaudeBuildArgs_BackendMetadata}, + } + + for _, tc := range suite { + t.Run(tc.name, tc.fn) + } } -func TestRunCleanupOldLogsKeepsCurrentProcessLog(t *testing.T) { +func TestLoggerCleanupOldLogsKeepsCurrentProcessLog(t *testing.T) { tempDir := setTempDirEnv(t, t.TempDir()) currentPID := os.Getpid() @@ -518,7 +628,7 @@ func TestRunCleanupOldLogsKeepsCurrentProcessLog(t *testing.T) { } } -func TestIsPIDReusedScenarios(t *testing.T) { +func TestLoggerIsPIDReusedScenarios(t *testing.T) { now := time.Now() tests := []struct { name string @@ -552,7 +662,7 @@ func TestIsPIDReusedScenarios(t *testing.T) { } } -func TestIsUnsafeFileSecurityChecks(t *testing.T) { +func TestLoggerIsUnsafeFileSecurityChecks(t *testing.T) { tempDir := t.TempDir() absTempDir, err := filepath.Abs(tempDir) if err != nil { @@ -601,7 +711,7 @@ func TestIsUnsafeFileSecurityChecks(t *testing.T) { }) } -func TestRunLoggerPathAndRemove(t *testing.T) { +func TestLoggerPathAndRemove(t *testing.T) { tempDir := t.TempDir() path := filepath.Join(tempDir, "sample.log") if err := os.WriteFile(path, []byte("test"), 0o644); err != nil { @@ -628,7 +738,19 @@ func TestRunLoggerPathAndRemove(t *testing.T) { } } -func TestRunLoggerInternalLog(t *testing.T) { +func TestLoggerTruncateBytesCoverage(t *testing.T) { + if got := truncateBytes([]byte("abc"), 3); got != "abc" { + t.Fatalf("truncateBytes() = %q, want %q", got, "abc") + } + if got := truncateBytes([]byte("abcd"), 3); got != "abc..." { + t.Fatalf("truncateBytes() = %q, want %q", got, "abc...") + } + if got := truncateBytes([]byte("abcd"), -1); got != "" { + t.Fatalf("truncateBytes() = %q, want empty string", got) + } +} + +func TestLoggerInternalLog(t *testing.T) { logger := &Logger{ ch: make(chan logEntry, 1), done: make(chan struct{}), @@ -644,7 +766,7 @@ func TestRunLoggerInternalLog(t *testing.T) { logger.log("INFO", "hello") entry := <-done - if entry.level != "INFO" || entry.msg != "hello" { + if entry.msg != "hello" { t.Fatalf("unexpected entry %+v", entry) } @@ -653,7 +775,7 @@ func TestRunLoggerInternalLog(t *testing.T) { close(logger.done) } -func TestRunParsePIDFromLog(t *testing.T) { +func TestLoggerParsePIDFromLog(t *testing.T) { hugePID := strconv.FormatInt(math.MaxInt64, 10) + "0" tests := []struct { name string @@ -769,69 +891,93 @@ func (f fakeFileInfo) ModTime() time.Time { return f.modTime } func (f fakeFileInfo) IsDir() bool { return false } func (f fakeFileInfo) Sys() interface{} { return nil } -func TestExtractRecentErrors(t *testing.T) { +func TestLoggerExtractRecentErrors(t *testing.T) { tests := []struct { name string - content string + logs []struct{ level, msg string } maxEntries int want []string }{ { name: "empty log", - content: "", + logs: nil, maxEntries: 10, want: nil, }, { name: "no errors", - content: `[2025-01-01 12:00:00.000] [PID:123] INFO: started -[2025-01-01 12:00:01.000] [PID:123] DEBUG: processing`, + logs: []struct{ level, msg string }{ + {"INFO", "started"}, + {"DEBUG", "processing"}, + }, maxEntries: 10, want: nil, }, { name: "single error", - content: `[2025-01-01 12:00:00.000] [PID:123] INFO: started -[2025-01-01 12:00:01.000] [PID:123] ERROR: something failed`, + logs: []struct{ level, msg string }{ + {"INFO", "started"}, + {"ERROR", "something failed"}, + }, maxEntries: 10, - want: []string{"[2025-01-01 12:00:01.000] [PID:123] ERROR: something failed"}, + want: []string{"something failed"}, }, { name: "error and warn", - content: `[2025-01-01 12:00:00.000] [PID:123] INFO: started -[2025-01-01 12:00:01.000] [PID:123] WARN: warning message -[2025-01-01 12:00:02.000] [PID:123] ERROR: error message`, + logs: []struct{ level, msg string }{ + {"INFO", "started"}, + {"WARN", "warning message"}, + {"ERROR", "error message"}, + }, maxEntries: 10, want: []string{ - "[2025-01-01 12:00:01.000] [PID:123] WARN: warning message", - "[2025-01-01 12:00:02.000] [PID:123] ERROR: error message", + "warning message", + "error message", }, }, { name: "truncate to max", - content: `[2025-01-01 12:00:00.000] [PID:123] ERROR: error 1 -[2025-01-01 12:00:01.000] [PID:123] ERROR: error 2 -[2025-01-01 12:00:02.000] [PID:123] ERROR: error 3 -[2025-01-01 12:00:03.000] [PID:123] ERROR: error 4 -[2025-01-01 12:00:04.000] [PID:123] ERROR: error 5`, + logs: []struct{ level, msg string }{ + {"ERROR", "error 1"}, + {"ERROR", "error 2"}, + {"ERROR", "error 3"}, + {"ERROR", "error 4"}, + {"ERROR", "error 5"}, + }, maxEntries: 3, want: []string{ - "[2025-01-01 12:00:02.000] [PID:123] ERROR: error 3", - "[2025-01-01 12:00:03.000] [PID:123] ERROR: error 4", - "[2025-01-01 12:00:04.000] [PID:123] ERROR: error 5", + "error 3", + "error 4", + "error 5", }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tempDir := t.TempDir() - logPath := filepath.Join(tempDir, "test.log") - if err := os.WriteFile(logPath, []byte(tt.content), 0o644); err != nil { - t.Fatalf("failed to write test log: %v", err) + logger, err := NewLoggerWithSuffix("extract-test") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + defer logger.Close() + defer logger.RemoveLogFile() + + // Write logs using logger methods + for _, entry := range tt.logs { + switch entry.level { + case "INFO": + logger.Info(entry.msg) + case "WARN": + logger.Warn(entry.msg) + case "ERROR": + logger.Error(entry.msg) + case "DEBUG": + logger.Debug(entry.msg) + } } - logger := &Logger{path: logPath} + logger.Flush() + got := logger.ExtractRecentErrors(tt.maxEntries) if len(got) != len(tt.want) { @@ -846,23 +992,137 @@ func TestExtractRecentErrors(t *testing.T) { } } -func TestExtractRecentErrorsNilLogger(t *testing.T) { +func TestLoggerExtractRecentErrorsNilLogger(t *testing.T) { var logger *Logger if got := logger.ExtractRecentErrors(10); got != nil { t.Fatalf("nil logger ExtractRecentErrors() should return nil, got %v", got) } } -func TestExtractRecentErrorsEmptyPath(t *testing.T) { +func TestLoggerExtractRecentErrorsEmptyPath(t *testing.T) { logger := &Logger{path: ""} if got := logger.ExtractRecentErrors(10); got != nil { t.Fatalf("empty path ExtractRecentErrors() should return nil, got %v", got) } } -func TestExtractRecentErrorsFileNotExist(t *testing.T) { +func TestLoggerExtractRecentErrorsFileNotExist(t *testing.T) { logger := &Logger{path: "/nonexistent/path/to/log.log"} if got := logger.ExtractRecentErrors(10); got != nil { t.Fatalf("nonexistent file ExtractRecentErrors() should return nil, got %v", got) } } + +func TestSanitizeLogSuffixNoDuplicates(t *testing.T) { + testCases := []string{ + "task", + "task.", + ".task", + "-task", + "task-", + "--task--", + "..task..", + } + + seen := make(map[string]string) + for _, input := range testCases { + result := sanitizeLogSuffix(input) + if result == "" { + t.Fatalf("sanitizeLogSuffix(%q) returned empty string", input) + } + + if prev, exists := seen[result]; exists { + t.Fatalf("collision detected: %q and %q both produce %q", input, prev, result) + } + seen[result] = input + + // Verify result is safe for file names + if strings.ContainsAny(result, "/\\:*?\"<>|") { + t.Fatalf("sanitizeLogSuffix(%q) = %q contains unsafe characters", input, result) + } + } +} + +func TestExtractRecentErrorsBoundaryCheck(t *testing.T) { + logger, err := NewLoggerWithSuffix("boundary-test") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + defer logger.Close() + defer logger.RemoveLogFile() + + // Write some errors + logger.Error("error 1") + logger.Warn("warn 1") + logger.Error("error 2") + logger.Flush() + + // Test zero + result := logger.ExtractRecentErrors(0) + if result != nil { + t.Fatalf("ExtractRecentErrors(0) should return nil, got %v", result) + } + + // Test negative + result = logger.ExtractRecentErrors(-5) + if result != nil { + t.Fatalf("ExtractRecentErrors(-5) should return nil, got %v", result) + } + + // Test positive still works + result = logger.ExtractRecentErrors(10) + if len(result) != 3 { + t.Fatalf("ExtractRecentErrors(10) expected 3 entries, got %d", len(result)) + } +} + +func TestErrorEntriesMaxLimit(t *testing.T) { + logger, err := NewLoggerWithSuffix("max-limit-test") + if err != nil { + t.Fatalf("NewLoggerWithSuffix() error = %v", err) + } + defer logger.Close() + defer logger.RemoveLogFile() + + // Write 150 error/warn entries + for i := 1; i <= 150; i++ { + if i%2 == 0 { + logger.Error(fmt.Sprintf("error-%03d", i)) + } else { + logger.Warn(fmt.Sprintf("warn-%03d", i)) + } + } + logger.Flush() + + // Extract all cached errors + result := logger.ExtractRecentErrors(200) // Request more than cache size + + // Should only have last 100 entries (entries 51-150 in sequence) + if len(result) != 100 { + t.Fatalf("expected 100 cached entries, got %d", len(result)) + } + + // Verify entries are the last 100 (entries 51-150) + if !strings.Contains(result[0], "051") { + t.Fatalf("first cached entry should be entry 51, got: %s", result[0]) + } + if !strings.Contains(result[99], "150") { + t.Fatalf("last cached entry should be entry 150, got: %s", result[99]) + } + + // Verify order is preserved - simplified logic + for i := 0; i < len(result)-1; i++ { + expectedNum := 51 + i + nextNum := 51 + i + 1 + + expectedEntry := fmt.Sprintf("%03d", expectedNum) + nextEntry := fmt.Sprintf("%03d", nextNum) + + if !strings.Contains(result[i], expectedEntry) { + t.Fatalf("entry at index %d should contain %s, got: %s", i, expectedEntry, result[i]) + } + if !strings.Contains(result[i+1], nextEntry) { + t.Fatalf("entry at index %d should contain %s, got: %s", i+1, nextEntry, result[i+1]) + } + } +} diff --git a/codeagent-wrapper/main.go b/codeagent-wrapper/main.go index 7d642e9..39ce81e 100644 --- a/codeagent-wrapper/main.go +++ b/codeagent-wrapper/main.go @@ -14,14 +14,15 @@ import ( ) const ( - version = "5.2.2" - defaultWorkdir = "." - defaultTimeout = 7200 // seconds - codexLogLineLimit = 1000 - stdinSpecialChars = "\n\\\"'`$" - stderrCaptureLimit = 4 * 1024 - defaultBackendName = "codex" - defaultCodexCommand = "codex" + version = "5.4.0" + defaultWorkdir = "." + defaultTimeout = 7200 // seconds (2 hours) + defaultCoverageTarget = 90.0 + codexLogLineLimit = 1000 + stdinSpecialChars = "\n\\\"'`$" + stderrCaptureLimit = 4 * 1024 + defaultBackendName = "codex" + defaultCodexCommand = "codex" // stdout close reasons stdoutCloseReasonWait = "wait-done" @@ -30,6 +31,8 @@ const ( stdoutDrainTimeout = 100 * time.Millisecond ) +var useASCIIMode = os.Getenv("CODEAGENT_ASCII_MODE") == "true" + // Test hooks for dependency injection var ( stdinReader io.Reader = os.Stdin @@ -175,6 +178,7 @@ func run() (exitCode int) { if parallelIndex != -1 { backendName := defaultBackendName + fullOutput := false var extras []string for i := 0; i < len(args); i++ { @@ -182,6 +186,8 @@ func run() (exitCode int) { switch { case arg == "--parallel": continue + case arg == "--full-output": + fullOutput = true case arg == "--backend": if i+1 >= len(args) { fmt.Fprintln(os.Stderr, "ERROR: --backend flag requires a value") @@ -202,11 +208,12 @@ func run() (exitCode int) { } if len(extras) > 0 { - fmt.Fprintln(os.Stderr, "ERROR: --parallel reads its task configuration from stdin; only --backend is allowed.") + fmt.Fprintln(os.Stderr, "ERROR: --parallel reads its task configuration from stdin; only --backend and --full-output are allowed.") fmt.Fprintln(os.Stderr, "Usage examples:") fmt.Fprintf(os.Stderr, " %s --parallel < tasks.txt\n", name) fmt.Fprintf(os.Stderr, " echo '...' | %s --parallel\n", name) fmt.Fprintf(os.Stderr, " %s --parallel <<'EOF'\n", name) + fmt.Fprintf(os.Stderr, " %s --parallel --full-output <<'EOF' # include full task output\n", name) return 1 } @@ -244,7 +251,33 @@ func run() (exitCode int) { } results := executeConcurrent(layers, timeoutSec) - fmt.Println(generateFinalOutput(results)) + + // Extract structured report fields from each result + for i := range results { + results[i].CoverageTarget = defaultCoverageTarget + if results[i].Message == "" { + continue + } + + lines := strings.Split(results[i].Message, "\n") + + // Coverage extraction + results[i].Coverage = extractCoverageFromLines(lines) + results[i].CoverageNum = extractCoverageNum(results[i].Coverage) + + // Files changed + results[i].FilesChanged = extractFilesChangedFromLines(lines) + + // Test results + results[i].TestsPassed, results[i].TestsFailed = extractTestResultsFromLines(lines) + + // Key output summary + results[i].KeyOutput = extractKeyOutputFromLines(lines, 150) + } + + // Default: summary mode (context-efficient) + // --full-output: legacy full output mode + fmt.Println(generateFinalOutputWithMode(results, !fullOutput)) exitCode = 0 for _, res := range results { @@ -447,16 +480,19 @@ Usage: %[1]s resume "task" [workdir] %[1]s resume - [workdir] %[1]s --parallel Run tasks in parallel (config from stdin) + %[1]s --parallel --full-output Run tasks in parallel with full output (legacy) %[1]s --version %[1]s --help Parallel mode examples: %[1]s --parallel < tasks.txt echo '...' | %[1]s --parallel + %[1]s --parallel --full-output < tasks.txt %[1]s --parallel <<'EOF' Environment Variables: - CODEX_TIMEOUT Timeout in milliseconds (default: 7200000) + CODEX_TIMEOUT Timeout in milliseconds (default: 7200000) + CODEAGENT_ASCII_MODE Use ASCII symbols instead of Unicode (PASS/WARN/FAIL) Exit Codes: 0 Success diff --git a/codeagent-wrapper/main_integration_test.go b/codeagent-wrapper/main_integration_test.go index a5083cd..ebfba97 100644 --- a/codeagent-wrapper/main_integration_test.go +++ b/codeagent-wrapper/main_integration_test.go @@ -46,10 +46,26 @@ func parseIntegrationOutput(t *testing.T, out string) integrationOutput { lines := strings.Split(out, "\n") var currentTask *TaskResult + inTaskResults := false for _, line := range lines { line = strings.TrimSpace(line) - if strings.HasPrefix(line, "Total:") { + + // Parse new format header: "X tasks | Y passed | Z failed" + if strings.Contains(line, "tasks |") && strings.Contains(line, "passed |") { + parts := strings.Split(line, "|") + for _, p := range parts { + p = strings.TrimSpace(p) + if strings.HasSuffix(p, "tasks") { + fmt.Sscanf(p, "%d tasks", &payload.Summary.Total) + } else if strings.HasSuffix(p, "passed") { + fmt.Sscanf(p, "%d passed", &payload.Summary.Success) + } else if strings.HasSuffix(p, "failed") { + fmt.Sscanf(p, "%d failed", &payload.Summary.Failed) + } + } + } else if strings.HasPrefix(line, "Total:") { + // Legacy format: "Total: X | Success: Y | Failed: Z" parts := strings.Split(line, "|") for _, p := range parts { p = strings.TrimSpace(p) @@ -61,13 +77,72 @@ func parseIntegrationOutput(t *testing.T, out string) integrationOutput { fmt.Sscanf(p, "Failed: %d", &payload.Summary.Failed) } } + } else if line == "## Task Results" { + inTaskResults = true + } else if line == "## Summary" { + // End of task results section + if currentTask != nil { + payload.Results = append(payload.Results, *currentTask) + currentTask = nil + } + inTaskResults = false + } else if inTaskResults && strings.HasPrefix(line, "### ") { + // New task: ### task-id ✓ 92% or ### task-id PASS 92% (ASCII mode) + if currentTask != nil { + payload.Results = append(payload.Results, *currentTask) + } + currentTask = &TaskResult{} + + taskLine := strings.TrimPrefix(line, "### ") + success, warning, failed := getStatusSymbols() + // Parse different formats + if strings.Contains(taskLine, " "+success) { + parts := strings.Split(taskLine, " "+success) + currentTask.TaskID = strings.TrimSpace(parts[0]) + currentTask.ExitCode = 0 + // Extract coverage if present + if len(parts) > 1 { + coveragePart := strings.TrimSpace(parts[1]) + if strings.HasSuffix(coveragePart, "%") { + currentTask.Coverage = coveragePart + } + } + } else if strings.Contains(taskLine, " "+warning) { + parts := strings.Split(taskLine, " "+warning) + currentTask.TaskID = strings.TrimSpace(parts[0]) + currentTask.ExitCode = 0 + } else if strings.Contains(taskLine, " "+failed) { + parts := strings.Split(taskLine, " "+failed) + currentTask.TaskID = strings.TrimSpace(parts[0]) + currentTask.ExitCode = 1 + } else { + currentTask.TaskID = taskLine + } + } else if currentTask != nil && inTaskResults { + // Parse task details + if strings.HasPrefix(line, "Exit code:") { + fmt.Sscanf(line, "Exit code: %d", ¤tTask.ExitCode) + } else if strings.HasPrefix(line, "Error:") { + currentTask.Error = strings.TrimPrefix(line, "Error: ") + } else if strings.HasPrefix(line, "Log:") { + currentTask.LogPath = strings.TrimSpace(strings.TrimPrefix(line, "Log:")) + } else if strings.HasPrefix(line, "Did:") { + currentTask.KeyOutput = strings.TrimSpace(strings.TrimPrefix(line, "Did:")) + } else if strings.HasPrefix(line, "Detail:") { + // Error detail for failed tasks + if currentTask.Message == "" { + currentTask.Message = strings.TrimSpace(strings.TrimPrefix(line, "Detail:")) + } + } } else if strings.HasPrefix(line, "--- Task:") { + // Legacy full output format if currentTask != nil { payload.Results = append(payload.Results, *currentTask) } currentTask = &TaskResult{} currentTask.TaskID = strings.TrimSuffix(strings.TrimPrefix(line, "--- Task: "), " ---") - } else if currentTask != nil { + } else if currentTask != nil && !inTaskResults { + // Legacy format parsing if strings.HasPrefix(line, "Status: SUCCESS") { currentTask.ExitCode = 0 } else if strings.HasPrefix(line, "Status: FAILED") { @@ -82,15 +157,11 @@ func parseIntegrationOutput(t *testing.T, out string) integrationOutput { currentTask.SessionID = strings.TrimPrefix(line, "Session: ") } else if strings.HasPrefix(line, "Log:") { currentTask.LogPath = strings.TrimSpace(strings.TrimPrefix(line, "Log:")) - } else if line != "" && !strings.HasPrefix(line, "===") && !strings.HasPrefix(line, "---") { - if currentTask.Message != "" { - currentTask.Message += "\n" - } - currentTask.Message += line } } } + // Handle last task if currentTask != nil { payload.Results = append(payload.Results, *currentTask) } @@ -343,9 +414,10 @@ task-beta` } for _, id := range []string{"alpha", "beta"} { - want := fmt.Sprintf("Log: %s", logPathFor(id)) - if !strings.Contains(output, want) { - t.Fatalf("parallel output missing %q for %s:\n%s", want, id, output) + // Summary mode shows log paths in table format, not "Log: xxx" + logPath := logPathFor(id) + if !strings.Contains(output, logPath) { + t.Fatalf("parallel output missing log path %q for %s:\n%s", logPath, id, output) } } } @@ -426,10 +498,11 @@ ok-d` t.Fatalf("expected startup banner in stderr, got:\n%s", stderrOut) } + // After parallel log isolation fix, each task has its own log file expectedLines := map[string]struct{}{ - fmt.Sprintf("Task a: Log: %s", expectedLog): {}, - fmt.Sprintf("Task b: Log: %s", expectedLog): {}, - fmt.Sprintf("Task d: Log: %s", expectedLog): {}, + fmt.Sprintf("Task a: Log: %s", filepath.Join(tempDir, fmt.Sprintf("codex-wrapper-%d-a.log", os.Getpid()))): {}, + fmt.Sprintf("Task b: Log: %s", filepath.Join(tempDir, fmt.Sprintf("codex-wrapper-%d-b.log", os.Getpid()))): {}, + fmt.Sprintf("Task d: Log: %s", filepath.Join(tempDir, fmt.Sprintf("codex-wrapper-%d-d.log", os.Getpid()))): {}, } if len(taskLines) != len(expectedLines) { @@ -549,16 +622,16 @@ ok-e` if resD.LogPath != logPathFor("D") || resE.LogPath != logPathFor("E") { t.Fatalf("expected log paths for D/E, got D=%q E=%q", resD.LogPath, resE.LogPath) } + // Summary mode shows log paths in table, verify they appear in output for _, id := range []string{"A", "D", "E"} { - block := extractTaskBlock(t, output, id) - want := fmt.Sprintf("Log: %s", logPathFor(id)) - if !strings.Contains(block, want) { - t.Fatalf("task %s block missing %q:\n%s", id, want, block) + logPath := logPathFor(id) + if !strings.Contains(output, logPath) { + t.Fatalf("task %s log path %q not found in output:\n%s", id, logPath, output) } } - blockB := extractTaskBlock(t, output, "B") - if strings.Contains(blockB, "Log:") { - t.Fatalf("skipped task B should not emit a log line:\n%s", blockB) + // Task B was skipped, should have "-" or empty log path in table + if resB.LogPath != "" { + t.Fatalf("skipped task B should have empty log path, got %q", resB.LogPath) } } diff --git a/codeagent-wrapper/main_test.go b/codeagent-wrapper/main_test.go index 6360464..b4ac34a 100644 --- a/codeagent-wrapper/main_test.go +++ b/codeagent-wrapper/main_test.go @@ -41,6 +41,7 @@ func resetTestHooks() { closeLogger() executablePathFn = os.Executable runTaskFn = runCodexTask + runCodexTaskFn = defaultRunCodexTaskFn exitFn = os.Exit } @@ -254,6 +255,10 @@ func (d *drainBlockingCmd) SetDir(dir string) { d.inner.SetDir(dir) } +func (d *drainBlockingCmd) SetEnv(env map[string]string) { + d.inner.SetEnv(env) +} + func (d *drainBlockingCmd) Process() processHandle { return d.inner.Process() } @@ -386,6 +391,8 @@ type fakeCmd struct { stderr io.Writer + env map[string]string + waitDelay time.Duration waitErr error startErr error @@ -510,6 +517,20 @@ func (f *fakeCmd) SetStderr(w io.Writer) { func (f *fakeCmd) SetDir(string) {} +func (f *fakeCmd) SetEnv(env map[string]string) { + if len(env) == 0 { + return + } + f.mu.Lock() + defer f.mu.Unlock() + if f.env == nil { + f.env = make(map[string]string, len(env)) + } + for k, v := range env { + f.env[k] = v + } +} + func (f *fakeCmd) Process() processHandle { if f == nil { return nil @@ -878,6 +899,79 @@ func TestRunCodexTask_ContextTimeout(t *testing.T) { } } +func TestRunCodexTask_ForcesStopAfterCompletion(t *testing.T) { + defer resetTestHooks() + forceKillDelay.Store(0) + + fake := newFakeCmd(fakeCmdConfig{ + StdoutPlan: []fakeStdoutEvent{ + {Data: `{"type":"item.completed","item":{"type":"agent_message","text":"done"}}` + "\n"}, + {Data: `{"type":"thread.completed","thread_id":"tid"}` + "\n"}, + }, + KeepStdoutOpen: true, + BlockWait: true, + ReleaseWaitOnSignal: true, + ReleaseWaitOnKill: true, + }) + + newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { + return fake + } + buildCodexArgsFn = func(cfg *Config, targetArg string) []string { return []string{targetArg} } + codexCommand = "fake-cmd" + + start := time.Now() + result := runCodexTaskWithContext(context.Background(), TaskSpec{Task: "done", WorkDir: defaultWorkdir}, nil, nil, false, false, 60) + duration := time.Since(start) + + if result.ExitCode != 0 || result.Message != "done" { + t.Fatalf("unexpected result: %+v", result) + } + if duration > 2*time.Second { + t.Fatalf("runCodexTaskWithContext took too long: %v", duration) + } + if fake.process.SignalCount() == 0 { + t.Fatalf("expected SIGTERM to be sent, got %d", fake.process.SignalCount()) + } +} + +func TestRunCodexTask_DoesNotTerminateBeforeThreadCompleted(t *testing.T) { + defer resetTestHooks() + forceKillDelay.Store(0) + + fake := newFakeCmd(fakeCmdConfig{ + StdoutPlan: []fakeStdoutEvent{ + {Data: `{"type":"item.completed","item":{"type":"agent_message","text":"intermediate"}}` + "\n"}, + {Delay: 1100 * time.Millisecond, Data: `{"type":"item.completed","item":{"type":"agent_message","text":"final"}}` + "\n"}, + {Data: `{"type":"thread.completed","thread_id":"tid"}` + "\n"}, + }, + KeepStdoutOpen: true, + BlockWait: true, + ReleaseWaitOnSignal: true, + ReleaseWaitOnKill: true, + }) + + newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner { + return fake + } + buildCodexArgsFn = func(cfg *Config, targetArg string) []string { return []string{targetArg} } + codexCommand = "fake-cmd" + + start := time.Now() + result := runCodexTaskWithContext(context.Background(), TaskSpec{Task: "done", WorkDir: defaultWorkdir}, nil, nil, false, false, 60) + duration := time.Since(start) + + if result.ExitCode != 0 || result.Message != "final" { + t.Fatalf("unexpected result: %+v", result) + } + if duration > 5*time.Second { + t.Fatalf("runCodexTaskWithContext took too long: %v", duration) + } + if fake.process.SignalCount() == 0 { + t.Fatalf("expected SIGTERM to be sent, got %d", fake.process.SignalCount()) + } +} + func TestBackendParseArgs_NewMode(t *testing.T) { tests := []struct { name string @@ -964,6 +1058,8 @@ func TestBackendParseArgs_ResumeMode(t *testing.T) { }, {name: "resume missing session_id", args: []string{"codeagent-wrapper", "resume"}, wantErr: true}, {name: "resume missing task", args: []string{"codeagent-wrapper", "resume", "session-123"}, wantErr: true}, + {name: "resume empty session_id", args: []string{"codeagent-wrapper", "resume", "", "task"}, wantErr: true}, + {name: "resume whitespace session_id", args: []string{"codeagent-wrapper", "resume", " ", "task"}, wantErr: true}, } for _, tt := range tests { @@ -1180,6 +1276,18 @@ do something` } } +func TestParallelParseConfig_EmptySessionID(t *testing.T) { + input := `---TASK--- +id: task-1 +session_id: +---CONTENT--- +do something` + + if _, err := parseParallelConfig([]byte(input)); err == nil { + t.Fatalf("expected error for empty session_id, got nil") + } +} + func TestParallelParseConfig_InvalidFormat(t *testing.T) { if _, err := parseParallelConfig([]byte("invalid format")); err == nil { t.Fatalf("expected error for invalid format, got nil") @@ -1280,9 +1388,19 @@ func TestRunShouldUseStdin(t *testing.T) { } func TestRunBuildCodexArgs_NewMode(t *testing.T) { + const key = "CODEX_BYPASS_SANDBOX" + t.Cleanup(func() { os.Unsetenv(key) }) + os.Unsetenv(key) + cfg := &Config{Mode: "new", WorkDir: "/test/dir"} args := buildCodexArgs(cfg, "my task") - expected := []string{"e", "--skip-git-repo-check", "-C", "/test/dir", "--json", "my task"} + expected := []string{ + "e", + "--skip-git-repo-check", + "-C", "/test/dir", + "--json", + "my task", + } if len(args) != len(expected) { t.Fatalf("len mismatch") } @@ -1294,9 +1412,20 @@ func TestRunBuildCodexArgs_NewMode(t *testing.T) { } func TestRunBuildCodexArgs_ResumeMode(t *testing.T) { + const key = "CODEX_BYPASS_SANDBOX" + t.Cleanup(func() { os.Unsetenv(key) }) + os.Unsetenv(key) + cfg := &Config{Mode: "resume", SessionID: "session-abc"} args := buildCodexArgs(cfg, "-") - expected := []string{"e", "--skip-git-repo-check", "--json", "resume", "session-abc", "-"} + expected := []string{ + "e", + "--skip-git-repo-check", + "--json", + "resume", + "session-abc", + "-", + } if len(args) != len(expected) { t.Fatalf("len mismatch") } @@ -1307,6 +1436,61 @@ func TestRunBuildCodexArgs_ResumeMode(t *testing.T) { } } +func TestRunBuildCodexArgs_ResumeMode_EmptySessionHandledGracefully(t *testing.T) { + const key = "CODEX_BYPASS_SANDBOX" + t.Cleanup(func() { os.Unsetenv(key) }) + os.Unsetenv(key) + + cfg := &Config{Mode: "resume", SessionID: " ", WorkDir: "/test/dir"} + args := buildCodexArgs(cfg, "task") + expected := []string{"e", "--skip-git-repo-check", "-C", "/test/dir", "--json", "task"} + if len(args) != len(expected) { + t.Fatalf("len mismatch") + } + for i := range args { + if args[i] != expected[i] { + t.Fatalf("args[%d]=%s, want %s", i, args[i], expected[i]) + } + } +} + +func TestRunBuildCodexArgs_BypassSandboxEnvTrue(t *testing.T) { + defer resetTestHooks() + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + + logger, err := NewLogger() + if err != nil { + t.Fatalf("NewLogger() error = %v", err) + } + setLogger(logger) + defer closeLogger() + + t.Setenv("CODEX_BYPASS_SANDBOX", "true") + + cfg := &Config{Mode: "new", WorkDir: "/test/dir"} + args := buildCodexArgs(cfg, "my task") + found := false + for _, arg := range args { + if arg == "--dangerously-bypass-approvals-and-sandbox" { + found = true + break + } + } + if !found { + t.Fatalf("expected bypass flag in args, got %v", args) + } + + logger.Flush() + data, err := os.ReadFile(logger.Path()) + if err != nil { + t.Fatalf("failed to read log file: %v", err) + } + if !strings.Contains(string(data), "CODEX_BYPASS_SANDBOX=true") { + t.Fatalf("expected bypass warning log, got: %s", string(data)) + } +} + func TestBackendSelectBackend(t *testing.T) { tests := []struct { name string @@ -1362,7 +1546,13 @@ func TestBackendBuildArgs_CodexBackend(t *testing.T) { backend := CodexBackend{} cfg := &Config{Mode: "new", WorkDir: "/test/dir"} got := backend.BuildArgs(cfg, "task") - want := []string{"e", "--skip-git-repo-check", "-C", "/test/dir", "--json", "task"} + want := []string{ + "e", + "--skip-git-repo-check", + "-C", "/test/dir", + "--json", + "task", + } if len(got) != len(want) { t.Fatalf("length mismatch") } @@ -1377,13 +1567,13 @@ func TestBackendBuildArgs_ClaudeBackend(t *testing.T) { backend := ClaudeBackend{} cfg := &Config{Mode: "new", WorkDir: defaultWorkdir} got := backend.BuildArgs(cfg, "todo") - want := []string{"-p", "--dangerously-skip-permissions", "--output-format", "stream-json", "--verbose", "todo"} + want := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose", "todo"} if len(got) != len(want) { - t.Fatalf("length mismatch") + t.Fatalf("args length=%d, want %d: %v", len(got), len(want), got) } for i := range want { if got[i] != want[i] { - t.Fatalf("index %d got %s want %s", i, got[i], want[i]) + t.Fatalf("index %d got %q want %q (args=%v)", i, got[i], want[i], got) } } @@ -1398,19 +1588,15 @@ func TestClaudeBackendBuildArgs_OutputValidation(t *testing.T) { target := "ensure-flags" args := backend.BuildArgs(cfg, target) - expectedPrefix := []string{"-p", "--dangerously-skip-permissions", "--output-format", "stream-json", "--verbose"} - - if len(args) != len(expectedPrefix)+1 { - t.Fatalf("args length=%d, want %d", len(args), len(expectedPrefix)+1) + want := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose", target} + if len(args) != len(want) { + t.Fatalf("args length=%d, want %d: %v", len(args), len(want), args) } - for i, val := range expectedPrefix { - if args[i] != val { - t.Fatalf("args[%d]=%q, want %q", i, args[i], val) + for i := range want { + if args[i] != want[i] { + t.Fatalf("index %d got %q want %q (args=%v)", i, args[i], want[i], args) } } - if args[len(args)-1] != target { - t.Fatalf("last arg=%q, want target %q", args[len(args)-1], target) - } } func TestBackendBuildArgs_GeminiBackend(t *testing.T) { @@ -1581,6 +1767,34 @@ func TestBackendParseJSONStream_ClaudeEvents(t *testing.T) { } } +func TestBackendParseJSONStream_ClaudeEvents_ItemDoesNotForceCodex(t *testing.T) { + tests := []struct { + name string + input string + }{ + { + name: "null item", + input: `{"type":"result","result":"OK","session_id":"abc123","item":null}`, + }, + { + name: "empty object item", + input: `{"type":"result","subtype":"x","result":"OK","session_id":"abc123","item":{}}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + message, threadID := parseJSONStream(strings.NewReader(tt.input)) + if message != "OK" { + t.Fatalf("message=%q, want %q", message, "OK") + } + if threadID != "abc123" { + t.Fatalf("threadID=%q, want %q", threadID, "abc123") + } + }) + } +} + func TestBackendParseJSONStream_GeminiEvents(t *testing.T) { input := `{"type":"init","session_id":"xyz789"} {"type":"message","role":"assistant","content":"Hi","delta":true,"session_id":"xyz789"} @@ -1597,6 +1811,43 @@ func TestBackendParseJSONStream_GeminiEvents(t *testing.T) { } } +func TestBackendParseJSONStream_GeminiEvents_DeltaFalseStillDetected(t *testing.T) { + input := `{"type":"init","session_id":"xyz789"} +{"type":"message","content":"Hi","delta":false,"session_id":"xyz789"} +{"type":"result","status":"success","session_id":"xyz789"}` + + message, threadID := parseJSONStream(strings.NewReader(input)) + + if message != "Hi" { + t.Fatalf("message=%q, want %q", message, "Hi") + } + if threadID != "xyz789" { + t.Fatalf("threadID=%q, want %q", threadID, "xyz789") + } +} + +func TestBackendParseJSONStream_GeminiEvents_OnMessageTriggeredOnStatus(t *testing.T) { + input := `{"type":"init","session_id":"xyz789"} +{"type":"message","role":"assistant","content":"Hi","delta":true,"session_id":"xyz789"} +{"type":"message","content":" there","delta":true} +{"type":"result","status":"success","session_id":"xyz789"}` + + var called int + message, threadID := parseJSONStreamInternal(strings.NewReader(input), nil, nil, func() { + called++ + }, nil) + + if message != "Hi there" { + t.Fatalf("message=%q, want %q", message, "Hi there") + } + if threadID != "xyz789" { + t.Fatalf("threadID=%q, want %q", threadID, "xyz789") + } + if called != 1 { + t.Fatalf("onMessage called=%d, want %d", called, 1) + } +} + func TestBackendParseJSONStreamWithWarn_InvalidLine(t *testing.T) { var warnings []string warnFn := func(msg string) { warnings = append(warnings, msg) } @@ -1613,7 +1864,7 @@ func TestBackendParseJSONStream_OnMessage(t *testing.T) { var called int message, threadID := parseJSONStreamInternal(strings.NewReader(`{"type":"item.completed","item":{"type":"agent_message","text":"hook"}}`), nil, nil, func() { called++ - }) + }, nil) if message != "hook" { t.Fatalf("message = %q, want hook", message) } @@ -1625,10 +1876,86 @@ func TestBackendParseJSONStream_OnMessage(t *testing.T) { } } +func TestBackendParseJSONStream_OnComplete_CodexThreadCompleted(t *testing.T) { + input := `{"type":"item.completed","item":{"type":"agent_message","text":"first"}}` + "\n" + + `{"type":"item.completed","item":{"type":"agent_message","text":"second"}}` + "\n" + + `{"type":"thread.completed","thread_id":"t-1"}` + + var onMessageCalls int + var onCompleteCalls int + message, threadID := parseJSONStreamInternal(strings.NewReader(input), nil, nil, func() { + onMessageCalls++ + }, func() { + onCompleteCalls++ + }) + if message != "second" { + t.Fatalf("message = %q, want second", message) + } + if threadID != "t-1" { + t.Fatalf("threadID = %q, want t-1", threadID) + } + if onMessageCalls != 2 { + t.Fatalf("onMessage calls = %d, want 2", onMessageCalls) + } + if onCompleteCalls != 1 { + t.Fatalf("onComplete calls = %d, want 1", onCompleteCalls) + } +} + +func TestBackendParseJSONStream_OnComplete_ClaudeResult(t *testing.T) { + input := `{"type":"message","subtype":"stream","session_id":"s-1"}` + "\n" + + `{"type":"result","result":"OK","session_id":"s-1"}` + + var onMessageCalls int + var onCompleteCalls int + message, threadID := parseJSONStreamInternal(strings.NewReader(input), nil, nil, func() { + onMessageCalls++ + }, func() { + onCompleteCalls++ + }) + if message != "OK" { + t.Fatalf("message = %q, want OK", message) + } + if threadID != "s-1" { + t.Fatalf("threadID = %q, want s-1", threadID) + } + if onMessageCalls != 1 { + t.Fatalf("onMessage calls = %d, want 1", onMessageCalls) + } + if onCompleteCalls != 1 { + t.Fatalf("onComplete calls = %d, want 1", onCompleteCalls) + } +} + +func TestBackendParseJSONStream_OnComplete_GeminiTerminalResultStatus(t *testing.T) { + input := `{"type":"message","role":"assistant","content":"Hi","delta":true,"session_id":"g-1"}` + "\n" + + `{"type":"result","status":"success","session_id":"g-1"}` + + var onMessageCalls int + var onCompleteCalls int + message, threadID := parseJSONStreamInternal(strings.NewReader(input), nil, nil, func() { + onMessageCalls++ + }, func() { + onCompleteCalls++ + }) + if message != "Hi" { + t.Fatalf("message = %q, want Hi", message) + } + if threadID != "g-1" { + t.Fatalf("threadID = %q, want g-1", threadID) + } + if onMessageCalls != 1 { + t.Fatalf("onMessage calls = %d, want 1", onMessageCalls) + } + if onCompleteCalls != 1 { + t.Fatalf("onComplete calls = %d, want 1", onCompleteCalls) + } +} + func TestBackendParseJSONStream_ScannerError(t *testing.T) { var warnings []string warnFn := func(msg string) { warnings = append(warnings, msg) } - message, threadID := parseJSONStreamInternal(errReader{err: errors.New("scan-fail")}, warnFn, nil, nil) + message, threadID := parseJSONStreamInternal(errReader{err: errors.New("scan-fail")}, warnFn, nil, nil, nil) if message != "" || threadID != "" { t.Fatalf("expected empty output on scanner error, got message=%q threadID=%q", message, threadID) } @@ -1783,13 +2110,13 @@ func TestRunLogFunctions(t *testing.T) { } output := string(data) - if !strings.Contains(output, "INFO: info message") { + if !strings.Contains(output, "info message") { t.Errorf("logInfo output missing, got: %s", output) } - if !strings.Contains(output, "WARN: warn message") { + if !strings.Contains(output, "warn message") { t.Errorf("logWarn output missing, got: %s", output) } - if !strings.Contains(output, "ERROR: error message") { + if !strings.Contains(output, "error message") { t.Errorf("logError output missing, got: %s", output) } } @@ -2306,14 +2633,17 @@ func TestRunGenerateFinalOutput(t *testing.T) { if out == "" { t.Fatalf("generateFinalOutput() returned empty string") } - if !strings.Contains(out, "Total: 3") || !strings.Contains(out, "Success: 2") || !strings.Contains(out, "Failed: 1") { + // New format: "X tasks | Y passed | Z failed" + if !strings.Contains(out, "3 tasks") || !strings.Contains(out, "2 passed") || !strings.Contains(out, "1 failed") { t.Fatalf("summary missing, got %q", out) } - if !strings.Contains(out, "Task: a") || !strings.Contains(out, "Task: b") { - t.Fatalf("task entries missing") + // New format uses ### task-id for each task + if !strings.Contains(out, "### a") || !strings.Contains(out, "### b") { + t.Fatalf("task entries missing in structured format") } - if strings.Contains(out, "Log:") { - t.Fatalf("unexpected log line when LogPath empty, got %q", out) + // Should have Summary section + if !strings.Contains(out, "## Summary") { + t.Fatalf("Summary section missing, got %q", out) } } @@ -2333,12 +2663,18 @@ func TestRunGenerateFinalOutput_LogPath(t *testing.T) { LogPath: "/tmp/log-b", }, } + // Test summary mode (default) - should contain log paths out := generateFinalOutput(results) - if !strings.Contains(out, "Session: sid\nLog: /tmp/log-a") { - t.Fatalf("output missing log line after session: %q", out) + if !strings.Contains(out, "/tmp/log-b") { + t.Fatalf("summary output missing log path for failed task: %q", out) + } + // Test full output mode - shows Session: and Log: lines + out = generateFinalOutputWithMode(results, false) + if !strings.Contains(out, "Session: sid") || !strings.Contains(out, "Log: /tmp/log-a") { + t.Fatalf("full output missing log line after session: %q", out) } if !strings.Contains(out, "Log: /tmp/log-b") { - t.Fatalf("output missing log line for failed task: %q", out) + t.Fatalf("full output missing log line for failed task: %q", out) } } @@ -2636,6 +2972,46 @@ test` } } +func TestRunParallelWithFullOutput(t *testing.T) { + defer resetTestHooks() + cleanupLogsFn = func() (CleanupStats, error) { return CleanupStats{}, nil } + + oldArgs := os.Args + t.Cleanup(func() { os.Args = oldArgs }) + os.Args = []string{"codeagent-wrapper", "--parallel", "--full-output"} + + stdinReader = strings.NewReader(`---TASK--- +id: T1 +---CONTENT--- +noop`) + t.Cleanup(func() { stdinReader = os.Stdin }) + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + return TaskResult{TaskID: task.ID, ExitCode: 0, Message: "full output marker"} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + out := captureOutput(t, func() { + if code := run(); code != 0 { + t.Fatalf("run exit = %d, want 0", code) + } + }) + + if !strings.Contains(out, "=== Parallel Execution Summary ===") { + t.Fatalf("output missing full-output header, got %q", out) + } + if !strings.Contains(out, "--- Task: T1 ---") { + t.Fatalf("output missing task block, got %q", out) + } + if !strings.Contains(out, "full output marker") { + t.Fatalf("output missing task message, got %q", out) + } + if strings.Contains(out, "=== Execution Report ===") { + t.Fatalf("output should not include summary-only header, got %q", out) + } +} + func TestParallelInvalidBackend(t *testing.T) { defer resetTestHooks() cleanupLogsFn = func() (CleanupStats, error) { return CleanupStats{}, nil } @@ -2690,7 +3066,9 @@ func TestVersionFlag(t *testing.T) { t.Errorf("exit = %d, want 0", code) } }) - want := "codeagent-wrapper version 5.2.2\n" + + want := "codeagent-wrapper version 5.4.0\n" + if output != want { t.Fatalf("output = %q, want %q", output, want) } @@ -2704,7 +3082,9 @@ func TestVersionShortFlag(t *testing.T) { t.Errorf("exit = %d, want 0", code) } }) - want := "codeagent-wrapper version 5.2.2\n" + + want := "codeagent-wrapper version 5.4.0\n" + if output != want { t.Fatalf("output = %q, want %q", output, want) } @@ -2718,7 +3098,9 @@ func TestVersionLegacyAlias(t *testing.T) { t.Errorf("exit = %d, want 0", code) } }) - want := "codex-wrapper version 5.2.2\n" + + want := "codex-wrapper version 5.4.0\n" + if output != want { t.Fatalf("output = %q, want %q", output, want) } @@ -3299,7 +3681,7 @@ func TestRun_PipedTaskReadError(t *testing.T) { if exitCode != 1 { t.Fatalf("exit=%d, want 1", exitCode) } - if !strings.Contains(logOutput, "ERROR: Failed to read piped stdin: read stdin: pipe failure") { + if !strings.Contains(logOutput, "Failed to read piped stdin: read stdin: pipe failure") { t.Fatalf("log missing piped read error, got %q", logOutput) } // Log file is always removed after completion (new behavior) diff --git a/codeagent-wrapper/parser.go b/codeagent-wrapper/parser.go index 7f97ff3..0718d21 100644 --- a/codeagent-wrapper/parser.go +++ b/codeagent-wrapper/parser.go @@ -50,12 +50,53 @@ func parseJSONStreamWithWarn(r io.Reader, warnFn func(string)) (message, threadI } func parseJSONStreamWithLog(r io.Reader, warnFn func(string), infoFn func(string)) (message, threadID string) { - return parseJSONStreamInternal(r, warnFn, infoFn, nil) + return parseJSONStreamInternal(r, warnFn, infoFn, nil, nil) } -func parseJSONStreamInternal(r io.Reader, warnFn func(string), infoFn func(string), onMessage func()) (message, threadID string) { - scanner := bufio.NewScanner(r) - scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) +const ( + jsonLineReaderSize = 64 * 1024 + jsonLineMaxBytes = 10 * 1024 * 1024 + jsonLinePreviewBytes = 256 +) + +type codexHeader struct { + Type string `json:"type"` + ThreadID string `json:"thread_id,omitempty"` + Item *struct { + Type string `json:"type"` + } `json:"item,omitempty"` +} + +// UnifiedEvent combines all backend event formats into a single structure +// to avoid multiple JSON unmarshal operations per event +type UnifiedEvent struct { + // Common fields + Type string `json:"type"` + + // Codex-specific fields + ThreadID string `json:"thread_id,omitempty"` + Item json.RawMessage `json:"item,omitempty"` // Lazy parse + + // Claude-specific fields + Subtype string `json:"subtype,omitempty"` + SessionID string `json:"session_id,omitempty"` + Result string `json:"result,omitempty"` + + // Gemini-specific fields + Role string `json:"role,omitempty"` + Content string `json:"content,omitempty"` + Delta *bool `json:"delta,omitempty"` + Status string `json:"status,omitempty"` +} + +// ItemContent represents the parsed item.text field for Codex events +type ItemContent struct { + Type string `json:"type"` + Text interface{} `json:"text"` +} + +func parseJSONStreamInternal(r io.Reader, warnFn func(string), infoFn func(string), onMessage func(), onComplete func()) (message, threadID string) { + reader := bufio.NewReaderSize(r, jsonLineReaderSize) if warnFn == nil { warnFn = func(string) {} @@ -70,6 +111,12 @@ func parseJSONStreamInternal(r io.Reader, warnFn func(string), infoFn func(strin } } + notifyComplete := func() { + if onComplete != nil { + onComplete() + } + } + totalEvents := 0 var ( @@ -78,51 +125,57 @@ func parseJSONStreamInternal(r io.Reader, warnFn func(string), infoFn func(strin geminiBuffer strings.Builder ) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line == "" { + for { + line, tooLong, err := readLineWithLimit(reader, jsonLineMaxBytes, jsonLinePreviewBytes) + if err != nil { + if errors.Is(err, io.EOF) { + break + } + warnFn("Read stdout error: " + err.Error()) + break + } + + line = bytes.TrimSpace(line) + if len(line) == 0 { continue } totalEvents++ - var raw map[string]json.RawMessage - if err := json.Unmarshal([]byte(line), &raw); err != nil { - warnFn(fmt.Sprintf("Failed to parse line: %s", truncate(line, 100))) + if tooLong { + warnFn(fmt.Sprintf("Skipped overlong JSON line (> %d bytes): %s", jsonLineMaxBytes, truncateBytes(line, 100))) continue } - hasItemType := false - if rawItem, ok := raw["item"]; ok { - var itemMap map[string]json.RawMessage - if err := json.Unmarshal(rawItem, &itemMap); err == nil { - if _, ok := itemMap["type"]; ok { - hasItemType = true - } - } + // Single unmarshal for all backend types + var event UnifiedEvent + if err := json.Unmarshal(line, &event); err != nil { + warnFn(fmt.Sprintf("Failed to parse event: %s", truncateBytes(line, 100))) + continue } - isCodex := hasItemType - if !isCodex { - if _, ok := raw["thread_id"]; ok { + // Detect backend type by field presence + isCodex := event.ThreadID != "" + if !isCodex && len(event.Item) > 0 { + var itemHeader struct { + Type string `json:"type"` + } + if json.Unmarshal(event.Item, &itemHeader) == nil && itemHeader.Type != "" { isCodex = true } } + isClaude := event.Subtype != "" || event.Result != "" + if !isClaude && event.Type == "result" && event.SessionID != "" && event.Status == "" { + isClaude = true + } + isGemini := event.Role != "" || event.Delta != nil || event.Status != "" - switch { - case isCodex: - var event JSONEvent - if err := json.Unmarshal([]byte(line), &event); err != nil { - warnFn(fmt.Sprintf("Failed to parse Codex event: %s", truncate(line, 100))) - continue - } - + // Handle Codex events + if isCodex { var details []string if event.ThreadID != "" { details = append(details, fmt.Sprintf("thread_id=%s", event.ThreadID)) } - if event.Item != nil && event.Item.Type != "" { - details = append(details, fmt.Sprintf("item_type=%s", event.Item.Type)) - } + if len(details) > 0 { infoFn(fmt.Sprintf("Parsed event #%d type=%s (%s)", totalEvents, event.Type, strings.Join(details, ", "))) } else { @@ -133,27 +186,47 @@ func parseJSONStreamInternal(r io.Reader, warnFn func(string), infoFn func(strin case "thread.started": threadID = event.ThreadID infoFn(fmt.Sprintf("thread.started event thread_id=%s", threadID)) + + case "thread.completed": + if event.ThreadID != "" && threadID == "" { + threadID = event.ThreadID + } + infoFn(fmt.Sprintf("thread.completed event thread_id=%s", event.ThreadID)) + notifyComplete() + case "item.completed": var itemType string - var normalized string - if event.Item != nil { - itemType = event.Item.Type - normalized = normalizeText(event.Item.Text) + if len(event.Item) > 0 { + var itemHeader struct { + Type string `json:"type"` + } + if err := json.Unmarshal(event.Item, &itemHeader); err == nil { + itemType = itemHeader.Type + } } - infoFn(fmt.Sprintf("item.completed event item_type=%s message_len=%d", itemType, len(normalized))) - if event.Item != nil && event.Item.Type == "agent_message" && normalized != "" { - codexMessage = normalized - notifyMessage() + + if itemType == "agent_message" && len(event.Item) > 0 { + // Lazy parse: only parse item content when needed + var item ItemContent + if err := json.Unmarshal(event.Item, &item); err == nil { + normalized := normalizeText(item.Text) + infoFn(fmt.Sprintf("item.completed event item_type=%s message_len=%d", itemType, len(normalized))) + if normalized != "" { + codexMessage = normalized + notifyMessage() + } + } else { + warnFn(fmt.Sprintf("Failed to parse item content: %s", err.Error())) + } + } else { + infoFn(fmt.Sprintf("item.completed event item_type=%s", itemType)) } } + continue + } - case hasKey(raw, "subtype") || hasKey(raw, "result"): - var event ClaudeEvent - if err := json.Unmarshal([]byte(line), &event); err != nil { - warnFn(fmt.Sprintf("Failed to parse Claude event: %s", truncate(line, 100))) - continue - } - + // Handle Claude events + if isClaude { if event.SessionID != "" && threadID == "" { threadID = event.SessionID } @@ -165,31 +238,41 @@ func parseJSONStreamInternal(r io.Reader, warnFn func(string), infoFn func(strin notifyMessage() } - case hasKey(raw, "role") || hasKey(raw, "delta"): - var event GeminiEvent - if err := json.Unmarshal([]byte(line), &event); err != nil { - warnFn(fmt.Sprintf("Failed to parse Gemini event: %s", truncate(line, 100))) - continue + if event.Type == "result" { + notifyComplete() } + continue + } + // Handle Gemini events + if isGemini { if event.SessionID != "" && threadID == "" { threadID = event.SessionID } if event.Content != "" { geminiBuffer.WriteString(event.Content) - notifyMessage() } - infoFn(fmt.Sprintf("Parsed Gemini event #%d type=%s role=%s delta=%t status=%s content_len=%d", totalEvents, event.Type, event.Role, event.Delta, event.Status, len(event.Content))) + if event.Status != "" { + notifyMessage() - default: - warnFn(fmt.Sprintf("Unknown event format: %s", truncate(line, 100))) + if event.Type == "result" && (event.Status == "success" || event.Status == "error" || event.Status == "complete" || event.Status == "failed") { + notifyComplete() + } + } + + delta := false + if event.Delta != nil { + delta = *event.Delta + } + + infoFn(fmt.Sprintf("Parsed Gemini event #%d type=%s role=%s delta=%t status=%s content_len=%d", totalEvents, event.Type, event.Role, delta, event.Status, len(event.Content))) + continue } - } - if err := scanner.Err(); err != nil && !errors.Is(err, io.EOF) { - warnFn("Read stdout error: " + err.Error()) + // Unknown event format + warnFn(fmt.Sprintf("Unknown event format: %s", truncateBytes(line, 100))) } switch { @@ -236,6 +319,79 @@ func discardInvalidJSON(decoder *json.Decoder, reader *bufio.Reader) (*bufio.Rea return bufio.NewReader(io.MultiReader(bytes.NewReader(remaining), reader)), err } +func readLineWithLimit(r *bufio.Reader, maxBytes int, previewBytes int) (line []byte, tooLong bool, err error) { + if r == nil { + return nil, false, errors.New("reader is nil") + } + if maxBytes <= 0 { + return nil, false, errors.New("maxBytes must be > 0") + } + if previewBytes < 0 { + previewBytes = 0 + } + + part, isPrefix, err := r.ReadLine() + if err != nil { + return nil, false, err + } + + if !isPrefix { + if len(part) > maxBytes { + return part[:min(len(part), previewBytes)], true, nil + } + return part, false, nil + } + + preview := make([]byte, 0, min(previewBytes, len(part))) + if previewBytes > 0 { + preview = append(preview, part[:min(previewBytes, len(part))]...) + } + + buf := make([]byte, 0, min(maxBytes, len(part)*2)) + total := 0 + if len(part) > maxBytes { + tooLong = true + } else { + buf = append(buf, part...) + total = len(part) + } + + for isPrefix { + part, isPrefix, err = r.ReadLine() + if err != nil { + return nil, tooLong, err + } + + if previewBytes > 0 && len(preview) < previewBytes { + preview = append(preview, part[:min(previewBytes-len(preview), len(part))]...) + } + + if !tooLong { + if total+len(part) > maxBytes { + tooLong = true + continue + } + buf = append(buf, part...) + total += len(part) + } + } + + if tooLong { + return preview, true, nil + } + return buf, false, nil +} + +func truncateBytes(b []byte, maxLen int) string { + if len(b) <= maxLen { + return string(b) + } + if maxLen < 0 { + return "" + } + return string(b[:maxLen]) + "..." +} + func normalizeText(text interface{}) string { switch v := text.(type) { case string: diff --git a/codeagent-wrapper/parser_token_too_long_test.go b/codeagent-wrapper/parser_token_too_long_test.go new file mode 100644 index 0000000..662e443 --- /dev/null +++ b/codeagent-wrapper/parser_token_too_long_test.go @@ -0,0 +1,31 @@ +package main + +import ( + "strings" + "testing" +) + +func TestParseJSONStream_SkipsOverlongLineAndContinues(t *testing.T) { + // Exceed the 10MB bufio.Scanner limit in parseJSONStreamInternal. + tooLong := strings.Repeat("a", 11*1024*1024) + + input := strings.Join([]string{ + `{"type":"item.completed","item":{"type":"other_type","text":"` + tooLong + `"}}`, + `{"type":"thread.started","thread_id":"t-1"}`, + `{"type":"item.completed","item":{"type":"agent_message","text":"ok"}}`, + }, "\n") + + var warns []string + warnFn := func(msg string) { warns = append(warns, msg) } + + gotMessage, gotThreadID := parseJSONStreamInternal(strings.NewReader(input), warnFn, nil, nil, nil) + if gotMessage != "ok" { + t.Fatalf("message=%q, want %q (warns=%v)", gotMessage, "ok", warns) + } + if gotThreadID != "t-1" { + t.Fatalf("threadID=%q, want %q (warns=%v)", gotThreadID, "t-1", warns) + } + if len(warns) == 0 || !strings.Contains(warns[0], "Skipped overlong JSON line") { + t.Fatalf("expected warning about overlong JSON line, got %v", warns) + } +} diff --git a/codeagent-wrapper/utils.go b/codeagent-wrapper/utils.go index 3f4fa89..79dcec1 100644 --- a/codeagent-wrapper/utils.go +++ b/codeagent-wrapper/utils.go @@ -75,9 +75,10 @@ func getEnv(key, defaultValue string) string { } type logWriter struct { - prefix string - maxLen int - buf bytes.Buffer + prefix string + maxLen int + buf bytes.Buffer + dropped bool } func newLogWriter(prefix string, maxLen int) *logWriter { @@ -94,12 +95,12 @@ func (lw *logWriter) Write(p []byte) (int, error) { total := len(p) for len(p) > 0 { if idx := bytes.IndexByte(p, '\n'); idx >= 0 { - lw.buf.Write(p[:idx]) + lw.writeLimited(p[:idx]) lw.logLine(true) p = p[idx+1:] continue } - lw.buf.Write(p) + lw.writeLimited(p) break } return total, nil @@ -117,21 +118,53 @@ func (lw *logWriter) logLine(force bool) { return } line := lw.buf.String() + dropped := lw.dropped + lw.dropped = false lw.buf.Reset() if line == "" && !force { return } - if lw.maxLen > 0 && len(line) > lw.maxLen { - cutoff := lw.maxLen - if cutoff > 3 { - line = line[:cutoff-3] + "..." - } else { - line = line[:cutoff] + if lw.maxLen > 0 { + if dropped { + if lw.maxLen > 3 { + line = line[:min(len(line), lw.maxLen-3)] + "..." + } else { + line = line[:min(len(line), lw.maxLen)] + } + } else if len(line) > lw.maxLen { + cutoff := lw.maxLen + if cutoff > 3 { + line = line[:cutoff-3] + "..." + } else { + line = line[:cutoff] + } } } logInfo(lw.prefix + line) } +func (lw *logWriter) writeLimited(p []byte) { + if lw == nil || len(p) == 0 { + return + } + if lw.maxLen <= 0 { + lw.buf.Write(p) + return + } + + remaining := lw.maxLen - lw.buf.Len() + if remaining <= 0 { + lw.dropped = true + return + } + if len(p) <= remaining { + lw.buf.Write(p) + return + } + lw.buf.Write(p[:remaining]) + lw.dropped = true +} + type tailBuffer struct { limit int data []byte @@ -172,6 +205,55 @@ func truncate(s string, maxLen int) string { return s[:maxLen] + "..." } +// safeTruncate safely truncates string to maxLen, avoiding panic and UTF-8 corruption. +func safeTruncate(s string, maxLen int) string { + if maxLen <= 0 || s == "" { + return "" + } + + runes := []rune(s) + if len(runes) <= maxLen { + return s + } + + if maxLen < 4 { + return string(runes[:1]) + } + + cutoff := maxLen - 3 + if cutoff <= 0 { + return string(runes[:1]) + } + if len(runes) <= cutoff { + return s + } + return string(runes[:cutoff]) + "..." +} + +// sanitizeOutput removes ANSI escape sequences and control characters. +func sanitizeOutput(s string) string { + var result strings.Builder + inEscape := false + for i := 0; i < len(s); i++ { + if s[i] == '\x1b' && i+1 < len(s) && s[i+1] == '[' { + inEscape = true + i++ // skip '[' + continue + } + if inEscape { + if (s[i] >= 'A' && s[i] <= 'Z') || (s[i] >= 'a' && s[i] <= 'z') { + inEscape = false + } + continue + } + // Keep printable chars and common whitespace. + if s[i] >= 32 || s[i] == '\n' || s[i] == '\t' { + result.WriteByte(s[i]) + } + } + return result.String() +} + func min(a, b int) int { if a < b { return a @@ -190,3 +272,444 @@ func greet(name string) string { func farewell(name string) string { return "goodbye " + name } + +// extractMessageSummary extracts a brief summary from task output +// Returns first meaningful line or truncated content up to maxLen chars +func extractMessageSummary(message string, maxLen int) string { + if message == "" || maxLen <= 0 { + return "" + } + + // Try to find a meaningful summary line + lines := strings.Split(message, "\n") + for _, line := range lines { + line = strings.TrimSpace(line) + // Skip empty lines and common noise + if line == "" || strings.HasPrefix(line, "```") || strings.HasPrefix(line, "---") { + continue + } + // Found a meaningful line + return safeTruncate(line, maxLen) + } + + // Fallback: truncate entire message + clean := strings.TrimSpace(message) + return safeTruncate(clean, maxLen) +} + +// extractCoverageFromLines extracts coverage from pre-split lines. +func extractCoverageFromLines(lines []string) string { + if len(lines) == 0 { + return "" + } + + end := len(lines) + for end > 0 && strings.TrimSpace(lines[end-1]) == "" { + end-- + } + + if end == 1 { + trimmed := strings.TrimSpace(lines[0]) + if strings.HasSuffix(trimmed, "%") { + if num, err := strconv.ParseFloat(strings.TrimSuffix(trimmed, "%"), 64); err == nil && num >= 0 && num <= 100 { + return trimmed + } + } + } + + coverageKeywords := []string{"file", "stmt", "branch", "line", "coverage", "total"} + + for _, line := range lines[:end] { + lower := strings.ToLower(line) + + hasKeyword := false + tokens := strings.FieldsFunc(lower, func(r rune) bool { return r < 'a' || r > 'z' }) + for _, token := range tokens { + for _, kw := range coverageKeywords { + if strings.HasPrefix(token, kw) { + hasKeyword = true + break + } + } + if hasKeyword { + break + } + } + if !hasKeyword { + continue + } + if !strings.Contains(line, "%") { + continue + } + + // Extract percentage pattern: number followed by % + for i := 0; i < len(line); i++ { + if line[i] == '%' && i > 0 { + // Walk back to find the number + j := i - 1 + for j >= 0 && (line[j] == '.' || (line[j] >= '0' && line[j] <= '9')) { + j-- + } + if j < i-1 { + numStr := line[j+1 : i] + // Validate it's a reasonable percentage + if num, err := strconv.ParseFloat(numStr, 64); err == nil && num >= 0 && num <= 100 { + return numStr + "%" + } + } + } + } + } + + return "" +} + +// extractCoverage extracts coverage percentage from task output +// Supports common formats: "Coverage: 92%", "92% coverage", "coverage 92%", "TOTAL 92%" +func extractCoverage(message string) string { + if message == "" { + return "" + } + + return extractCoverageFromLines(strings.Split(message, "\n")) +} + +// extractCoverageNum extracts coverage as a numeric value for comparison +func extractCoverageNum(coverage string) float64 { + if coverage == "" { + return 0 + } + // Remove % sign and parse + numStr := strings.TrimSuffix(coverage, "%") + if num, err := strconv.ParseFloat(numStr, 64); err == nil { + return num + } + return 0 +} + +// extractFilesChangedFromLines extracts files from pre-split lines. +func extractFilesChangedFromLines(lines []string) []string { + if len(lines) == 0 { + return nil + } + + var files []string + seen := make(map[string]bool) + exts := []string{".ts", ".tsx", ".js", ".jsx", ".go", ".py", ".rs", ".java", ".vue", ".css", ".scss", ".md", ".json", ".yaml", ".yml", ".toml"} + + for _, line := range lines { + line = strings.TrimSpace(line) + + // Pattern 1: "Modified: path/to/file.ts" or "Created: path/to/file.ts" + matchedPrefix := false + for _, prefix := range []string{"Modified:", "Created:", "Updated:", "Edited:", "Wrote:", "Changed:"} { + if strings.HasPrefix(line, prefix) { + file := strings.TrimSpace(strings.TrimPrefix(line, prefix)) + file = strings.Trim(file, "`,\"'()[],:") + file = strings.TrimPrefix(file, "@") + if file != "" && !seen[file] { + files = append(files, file) + seen[file] = true + } + matchedPrefix = true + break + } + } + if matchedPrefix { + continue + } + + // Pattern 2: Tokens that look like file paths (allow root files, strip @ prefix). + parts := strings.Fields(line) + for _, part := range parts { + part = strings.Trim(part, "`,\"'()[],:") + part = strings.TrimPrefix(part, "@") + for _, ext := range exts { + if strings.HasSuffix(part, ext) && !seen[part] { + files = append(files, part) + seen[part] = true + break + } + } + } + } + + // Limit to first 10 files to avoid bloat + if len(files) > 10 { + files = files[:10] + } + + return files +} + +// extractFilesChanged extracts list of changed files from task output +// Looks for common patterns like "Modified: file.ts", "Created: file.ts", file paths in output +func extractFilesChanged(message string) []string { + if message == "" { + return nil + } + + return extractFilesChangedFromLines(strings.Split(message, "\n")) +} + +// extractTestResultsFromLines extracts test results from pre-split lines. +func extractTestResultsFromLines(lines []string) (passed, failed int) { + if len(lines) == 0 { + return 0, 0 + } + + // Common patterns: + // pytest: "12 passed, 2 failed" + // jest: "Tests: 2 failed, 12 passed" + // go: "ok ... 12 tests" + + for _, line := range lines { + line = strings.ToLower(line) + + // Look for test result lines + if !strings.Contains(line, "pass") && !strings.Contains(line, "fail") && !strings.Contains(line, "test") { + continue + } + + // Extract numbers near "passed" or "pass" + if idx := strings.Index(line, "pass"); idx != -1 { + // Look for number before "pass" + num := extractNumberBefore(line, idx) + if num > 0 { + passed = num + } + } + + // Extract numbers near "failed" or "fail" + if idx := strings.Index(line, "fail"); idx != -1 { + num := extractNumberBefore(line, idx) + if num > 0 { + failed = num + } + } + + // go test style: "ok ... 12 tests" + if passed == 0 { + if idx := strings.Index(line, "test"); idx != -1 { + num := extractNumberBefore(line, idx) + if num > 0 { + passed = num + } + } + } + + // If we found both, stop + if passed > 0 && failed > 0 { + break + } + } + + return passed, failed +} + +// extractTestResults extracts test pass/fail counts from task output +func extractTestResults(message string) (passed, failed int) { + if message == "" { + return 0, 0 + } + + return extractTestResultsFromLines(strings.Split(message, "\n")) +} + +// extractNumberBefore extracts a number that appears before the given index +func extractNumberBefore(s string, idx int) int { + if idx <= 0 { + return 0 + } + + // Walk backwards to find digits + end := idx - 1 + for end >= 0 && (s[end] == ' ' || s[end] == ':' || s[end] == ',') { + end-- + } + if end < 0 { + return 0 + } + + start := end + for start >= 0 && s[start] >= '0' && s[start] <= '9' { + start-- + } + start++ + + if start > end { + return 0 + } + + numStr := s[start : end+1] + if num, err := strconv.Atoi(numStr); err == nil { + return num + } + return 0 +} + +// extractKeyOutputFromLines extracts key output from pre-split lines. +func extractKeyOutputFromLines(lines []string, maxLen int) string { + if len(lines) == 0 || maxLen <= 0 { + return "" + } + + // Priority 1: Look for explicit summary lines + for _, line := range lines { + line = strings.TrimSpace(line) + lower := strings.ToLower(line) + if strings.HasPrefix(lower, "summary:") || strings.HasPrefix(lower, "completed:") || + strings.HasPrefix(lower, "implemented:") || strings.HasPrefix(lower, "added:") || + strings.HasPrefix(lower, "created:") || strings.HasPrefix(lower, "fixed:") { + content := line + for _, prefix := range []string{"Summary:", "Completed:", "Implemented:", "Added:", "Created:", "Fixed:", + "summary:", "completed:", "implemented:", "added:", "created:", "fixed:"} { + content = strings.TrimPrefix(content, prefix) + } + content = strings.TrimSpace(content) + if len(content) > 0 { + return safeTruncate(content, maxLen) + } + } + } + + // Priority 2: First meaningful line (skip noise) + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "```") || strings.HasPrefix(line, "---") || + strings.HasPrefix(line, "#") || strings.HasPrefix(line, "//") { + continue + } + // Skip very short lines (likely headers or markers) + if len(line) < 20 { + continue + } + return safeTruncate(line, maxLen) + } + + // Fallback: truncate entire message + clean := strings.TrimSpace(strings.Join(lines, "\n")) + return safeTruncate(clean, maxLen) +} + +// extractKeyOutput extracts a brief summary of what the task accomplished +// Looks for summary lines, first meaningful sentence, or truncates message +func extractKeyOutput(message string, maxLen int) string { + if message == "" || maxLen <= 0 { + return "" + } + return extractKeyOutputFromLines(strings.Split(message, "\n"), maxLen) +} + +// extractCoverageGap extracts what's missing from coverage reports +// Looks for uncovered lines, branches, or functions +func extractCoverageGap(message string) string { + if message == "" { + return "" + } + + lower := strings.ToLower(message) + lines := strings.Split(message, "\n") + + // Look for uncovered/missing patterns + for _, line := range lines { + lineLower := strings.ToLower(line) + line = strings.TrimSpace(line) + + // Common patterns for uncovered code + if strings.Contains(lineLower, "uncovered") || + strings.Contains(lineLower, "not covered") || + strings.Contains(lineLower, "missing coverage") || + strings.Contains(lineLower, "lines not covered") { + if len(line) > 100 { + return line[:97] + "..." + } + return line + } + + // Look for specific file:line patterns in coverage reports + if strings.Contains(lineLower, "branch") && strings.Contains(lineLower, "not taken") { + if len(line) > 100 { + return line[:97] + "..." + } + return line + } + } + + // Look for function names that aren't covered + if strings.Contains(lower, "function") && strings.Contains(lower, "0%") { + for _, line := range lines { + if strings.Contains(strings.ToLower(line), "0%") && strings.Contains(line, "function") { + line = strings.TrimSpace(line) + if len(line) > 100 { + return line[:97] + "..." + } + return line + } + } + } + + return "" +} + +// extractErrorDetail extracts meaningful error context from task output +// Returns the most relevant error information up to maxLen characters +func extractErrorDetail(message string, maxLen int) string { + if message == "" || maxLen <= 0 { + return "" + } + + lines := strings.Split(message, "\n") + var errorLines []string + + // Look for error-related lines + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + + lower := strings.ToLower(line) + + // Skip noise lines + if strings.HasPrefix(line, "at ") && strings.Contains(line, "(") { + // Stack trace line - only keep first one + if len(errorLines) > 0 && strings.HasPrefix(strings.ToLower(errorLines[len(errorLines)-1]), "at ") { + continue + } + } + + // Prioritize error/fail lines + if strings.Contains(lower, "error") || + strings.Contains(lower, "fail") || + strings.Contains(lower, "exception") || + strings.Contains(lower, "assert") || + strings.Contains(lower, "expected") || + strings.Contains(lower, "timeout") || + strings.Contains(lower, "not found") || + strings.Contains(lower, "cannot") || + strings.Contains(lower, "undefined") || + strings.HasPrefix(line, "FAIL") || + strings.HasPrefix(line, "●") { + errorLines = append(errorLines, line) + } + } + + if len(errorLines) == 0 { + // No specific error lines found, take last few lines + start := len(lines) - 5 + if start < 0 { + start = 0 + } + for _, line := range lines[start:] { + line = strings.TrimSpace(line) + if line != "" { + errorLines = append(errorLines, line) + } + } + } + + // Join and truncate + result := strings.Join(errorLines, " | ") + return safeTruncate(result, maxLen) +} diff --git a/codeagent-wrapper/utils_test.go b/codeagent-wrapper/utils_test.go new file mode 100644 index 0000000..98a7427 --- /dev/null +++ b/codeagent-wrapper/utils_test.go @@ -0,0 +1,143 @@ +package main + +import ( + "fmt" + "reflect" + "strings" + "testing" +) + +func TestExtractCoverage(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"bare int", "92%", "92%"}, + {"bare float", "92.5%", "92.5%"}, + {"coverage prefix", "coverage: 92%", "92%"}, + {"total prefix", "TOTAL 92%", "92%"}, + {"all files", "All files 92%", "92%"}, + {"empty", "", ""}, + {"no number", "coverage: N/A", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := extractCoverage(tt.in); got != tt.want { + t.Fatalf("extractCoverage(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestExtractTestResults(t *testing.T) { + tests := []struct { + name string + in string + wantPassed int + wantFailed int + }{ + {"pytest one line", "12 passed, 2 failed", 12, 2}, + {"pytest split lines", "12 passed\n2 failed", 12, 2}, + {"jest format", "Tests: 2 failed, 12 passed, 14 total", 12, 2}, + {"go test style count", "ok\texample.com/foo\t0.12s\t12 tests", 12, 0}, + {"zero counts", "0 passed, 0 failed", 0, 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + passed, failed := extractTestResults(tt.in) + if passed != tt.wantPassed || failed != tt.wantFailed { + t.Fatalf("extractTestResults(%q) = (%d, %d), want (%d, %d)", tt.in, passed, failed, tt.wantPassed, tt.wantFailed) + } + }) + } +} + +func TestExtractFilesChanged(t *testing.T) { + tests := []struct { + name string + in string + want []string + }{ + {"root file", "Modified: main.go\n", []string{"main.go"}}, + {"path file", "Created: codeagent-wrapper/utils.go\n", []string{"codeagent-wrapper/utils.go"}}, + {"at prefix", "Updated: @codeagent-wrapper/main.go\n", []string{"codeagent-wrapper/main.go"}}, + {"token scan", "Files: @main.go, @codeagent-wrapper/utils.go\n", []string{"main.go", "codeagent-wrapper/utils.go"}}, + {"space path", "Modified: dir/with space/file.go\n", []string{"dir/with space/file.go"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := extractFilesChanged(tt.in); !reflect.DeepEqual(got, tt.want) { + t.Fatalf("extractFilesChanged(%q) = %#v, want %#v", tt.in, got, tt.want) + } + }) + } + + t.Run("limits to first 10", func(t *testing.T) { + var b strings.Builder + for i := 0; i < 12; i++ { + fmt.Fprintf(&b, "Modified: file%d.go\n", i) + } + got := extractFilesChanged(b.String()) + if len(got) != 10 { + t.Fatalf("len(files)=%d, want 10: %#v", len(got), got) + } + for i := 0; i < 10; i++ { + want := fmt.Sprintf("file%d.go", i) + if got[i] != want { + t.Fatalf("files[%d]=%q, want %q", i, got[i], want) + } + } + }) +} + +func TestSafeTruncate(t *testing.T) { + tests := []struct { + name string + in string + maxLen int + want string + }{ + {"empty", "", 4, ""}, + {"zero maxLen", "hello", 0, ""}, + {"one rune", "你好", 1, "你"}, + {"two runes no truncate", "你好", 2, "你好"}, + {"three runes no truncate", "你好", 3, "你好"}, + {"two runes truncates long", "你好世界", 2, "你"}, + {"three runes truncates long", "你好世界", 3, "你"}, + {"four with ellipsis", "你好世界啊", 4, "你..."}, + {"emoji", "🙂🙂🙂🙂🙂", 4, "🙂..."}, + {"no truncate", "你好世界", 4, "你好世界"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := safeTruncate(tt.in, tt.maxLen); got != tt.want { + t.Fatalf("safeTruncate(%q, %d) = %q, want %q", tt.in, tt.maxLen, got, tt.want) + } + }) + } +} + +func TestSanitizeOutput(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"ansi", "\x1b[31mred\x1b[0m", "red"}, + {"control chars", "a\x07b\r\nc\t", "ab\nc\t"}, + {"normal", "hello\nworld\t!", "hello\nworld\t!"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sanitizeOutput(tt.in); got != tt.want { + t.Fatalf("sanitizeOutput(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} diff --git a/dev-workflow/agents/dev-plan-generator.md b/dev-workflow/agents/dev-plan-generator.md index 3df8c03..eaaf657 100644 --- a/dev-workflow/agents/dev-plan-generator.md +++ b/dev-workflow/agents/dev-plan-generator.md @@ -1,6 +1,6 @@ --- name: dev-plan-generator -description: Use this agent when you need to generate a structured development plan document (`dev-plan.md`) that breaks down a feature into concrete implementation tasks with testing requirements and acceptance criteria. This agent should be called after requirements analysis and before actual implementation begins.\n\n\nContext: User is orchestrating a feature development workflow and needs to create a development plan after Codex analysis is complete.\nuser: "Create a development plan for the user authentication feature based on the requirements and analysis"\nassistant: "I'll use the Task tool to launch the dev-plan-generator agent to create the structured development plan document."\n\nThe user needs a dev-plan.md document generated from requirements and analysis. Use the dev-plan-generator agent to create the structured task breakdown.\n\n\n\n\nContext: Orchestrator has completed requirements gathering and Codex analysis for a new feature and needs to generate the development plan before moving to implementation.\nuser: "We've completed the analysis for the payment integration feature. Generate the development plan."\nassistant: "I'm going to use the Task tool to launch the dev-plan-generator agent to create the dev-plan.md document with task breakdown and testing requirements."\n\nThis is the step in the workflow where the development plan document needs to be generated. Use the dev-plan-generator agent to create the structured plan.\n\n\n\n\nContext: User is working through a requirements-driven workflow and has just approved the technical specifications.\nuser: "The specs look good. Let's move forward with creating the implementation plan."\nassistant: "I'll use the Task tool to launch the dev-plan-generator agent to generate the dev-plan.md document with the task breakdown."\n\nAfter spec approval, the next step is generating the development plan. Use the dev-plan-generator agent to create the structured document.\n\n +description: Use this agent when you need to generate a structured development plan document (`dev-plan.md`) that breaks down a feature into concrete implementation tasks with testing requirements and acceptance criteria. This agent should be called after requirements analysis and before actual implementation begins.\n\n\nContext: User is orchestrating a feature development workflow and needs to create a development plan after codeagent analysis is complete.\nuser: "Create a development plan for the user authentication feature based on the requirements and analysis"\nassistant: "I'll use the Task tool to launch the dev-plan-generator agent to create the structured development plan document."\n\nThe user needs a dev-plan.md document generated from requirements and analysis. Use the dev-plan-generator agent to create the structured task breakdown.\n\n\n\n\nContext: Orchestrator has completed requirements gathering and codeagent analysis for a new feature and needs to generate the development plan before moving to implementation.\nuser: "We've completed the analysis for the payment integration feature. Generate the development plan."\nassistant: "I'm going to use the Task tool to launch the dev-plan-generator agent to create the dev-plan.md document with task breakdown and testing requirements."\n\nThis is the step in the workflow where the development plan document needs to be generated. Use the dev-plan-generator agent to create the structured plan.\n\n\n\n\nContext: User is working through a requirements-driven workflow and has just approved the technical specifications.\nuser: "The specs look good. Let's move forward with creating the implementation plan."\nassistant: "I'll use the Task tool to launch the dev-plan-generator agent to generate the dev-plan.md document with the task breakdown."\n\nAfter spec approval, the next step is generating the development plan. Use the dev-plan-generator agent to create the structured document.\n\n tools: Glob, Grep, Read, Edit, Write, TodoWrite model: sonnet color: green diff --git a/dev-workflow/commands/dev.md b/dev-workflow/commands/dev.md index 063d7c0..c99c6bb 100644 --- a/dev-workflow/commands/dev.md +++ b/dev-workflow/commands/dev.md @@ -2,7 +2,6 @@ description: Extreme lightweight end-to-end development workflow with requirements clarification, intelligent backend selection, parallel codeagent execution, and mandatory 90% test coverage --- - You are the /dev Workflow Orchestrator, an expert development workflow manager specializing in orchestrating minimal, efficient end-to-end development processes with parallel task execution and rigorous test coverage validation. --- @@ -27,7 +26,7 @@ These rules have HIGHEST PRIORITY and override all other instructions: - Orchestrate a streamlined 7-step development workflow (Step 0 + Step 1–6): 0. Backend selection (user constrained) 1. Requirement clarification through targeted questioning - 2. Technical analysis using codeagent + 2. Technical analysis using codeagent-wrapper 3. Development documentation generation 4. Parallel development execution (backend routing per task type) 5. Coverage validation (≥90% requirement) @@ -48,8 +47,9 @@ These rules have HIGHEST PRIORITY and override all other instructions: - MUST use AskUserQuestion tool - Focus questions on functional boundaries, inputs/outputs, constraints, testing, and required unit-test coverage levels - Iterate 2-3 rounds until clear; rely on judgment; keep questions concise + - After clarification complete: MUST use TodoWrite to create task tracking list with workflow steps -- **Step 2: codeagent Deep Analysis (Plan Mode Style)** +- **Step 2: codeagent-wrapper Deep Analysis (Plan Mode Style) [USE CODEAGENT-WRAPPER ONLY]** MUST use Bash tool to invoke `codeagent-wrapper` for deep analysis. Do NOT use Read/Glob/Grep tools directly - delegate all exploration to codeagent-wrapper. @@ -87,7 +87,7 @@ These rules have HIGHEST PRIORITY and override all other instructions: - During analysis, output whether the task needs UI work (yes/no) and the evidence - UI criteria: presence of style assets (.css, .scss, styled-components, CSS modules, tailwindcss) OR frontend component files (.tsx, .jsx, .vue) - **What codeagent Does in Analysis Mode**: + **What the AI backend does in Analysis Mode** (when invoked via codeagent-wrapper): 1. **Explore Codebase**: Use Glob, Grep, Read to understand structure, patterns, architecture 2. **Identify Existing Patterns**: Find how similar features are implemented, reuse conventions 3. **Evaluate Options**: When multiple approaches exist, list trade-offs (complexity, performance, security, maintainability) @@ -162,7 +162,6 @@ These rules have HIGHEST PRIORITY and override all other instructions: Scope: [task file scope] Test: [test command] Deliverables: code + unit tests + coverage ≥90% + coverage summary - EOF ---TASK--- id: [task-id-2] @@ -177,7 +176,7 @@ These rules have HIGHEST PRIORITY and override all other instructions: Deliverables: code + unit tests + coverage ≥90% + coverage summary EOF ``` - + - **Note**: Use `workdir: .` (current directory) for all tasks unless specific subdirectory is required - Execute independent tasks concurrently; serialize conflicting ones; track coverage reports - Backend is routed deterministically based on task `type`, no manual intervention needed diff --git a/docs/CODEAGENT-WRAPPER.md b/docs/CODEAGENT-WRAPPER.md index e8dc398..f8a589d 100644 --- a/docs/CODEAGENT-WRAPPER.md +++ b/docs/CODEAGENT-WRAPPER.md @@ -105,6 +105,7 @@ EOF Execute multiple tasks concurrently with dependency management: ```bash +# Default: summary output (context-efficient, recommended) codeagent-wrapper --parallel <<'EOF' ---TASK--- id: backend_1701234567 @@ -125,6 +126,47 @@ dependencies: backend_1701234567, frontend_1701234568 ---CONTENT--- add integration tests for user management flow EOF + +# Full output mode (for debugging, includes complete task messages) +codeagent-wrapper --parallel --full-output <<'EOF' +... +EOF +``` + +**Output Modes:** +- **Summary (default)**: Structured report with extracted `Did/Files/Tests/Coverage`, plus a short action summary. +- **Full (`--full-output`)**: Complete task messages included. Use only for debugging. + +**Summary Output Example:** +``` +=== Execution Report === +3 tasks | 2 passed | 1 failed | 1 below 90% + +## Task Results + +### backend_api ✓ 92% +Did: Implemented /api/users CRUD endpoints +Files: backend/users.go, backend/router.go +Tests: 12 passed +Log: /tmp/codeagent-xxx.log + +### frontend_form ⚠️ 88% (below 90%) +Did: Created login form with validation +Files: frontend/LoginForm.tsx +Tests: 8 passed +Gap: lines not covered: frontend/LoginForm.tsx:42-47 +Log: /tmp/codeagent-yyy.log + +### integration_tests ✗ FAILED +Exit code: 1 +Error: Assertion failed at line 45 +Detail: Expected status 200 but got 401 +Log: /tmp/codeagent-zzz.log + +## Summary +- 2/3 completed successfully +- Fix: integration_tests (Assertion failed at line 45) +- Coverage: frontend_form ``` **Parallel Task Format:** diff --git a/go.work b/go.work new file mode 100644 index 0000000..3644132 --- /dev/null +++ b/go.work @@ -0,0 +1,5 @@ +go 1.21 + +use ( + ./codeagent-wrapper +) diff --git a/hooks/pre-commit.sh b/hooks/pre-commit.sh index 282fa2f..0336ac9 100755 --- a/hooks/pre-commit.sh +++ b/hooks/pre-commit.sh @@ -5,7 +5,7 @@ set -e # Get staged files -STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM) +STAGED_FILES="$(git diff --cached --name-only --diff-filter=ACM)" if [ -z "$STAGED_FILES" ]; then echo "No files to validate" @@ -15,17 +15,32 @@ fi echo "Running pre-commit checks..." # Check Go files -GO_FILES=$(echo "$STAGED_FILES" | grep '\.go$' || true) +GO_FILES="$(printf '%s\n' "$STAGED_FILES" | grep '\.go$' || true)" if [ -n "$GO_FILES" ]; then echo "Checking Go files..." + if ! command -v gofmt &> /dev/null; then + echo "❌ gofmt not found. Please install Go (gofmt is included with the Go toolchain)." + exit 1 + fi + # Format check - gofmt -l $GO_FILES | while read -r file; do + GO_FILE_ARGS=() + while IFS= read -r file; do if [ -n "$file" ]; then - echo "❌ $file needs formatting (run: gofmt -w $file)" + GO_FILE_ARGS+=("$file") + fi + done <<< "$GO_FILES" + + if [ "${#GO_FILE_ARGS[@]}" -gt 0 ]; then + UNFORMATTED="$(gofmt -l "${GO_FILE_ARGS[@]}")" + if [ -n "$UNFORMATTED" ]; then + echo "❌ The following files need formatting:" + echo "$UNFORMATTED" + echo "Run: gofmt -w " exit 1 fi - done + fi # Run tests if command -v go &> /dev/null; then @@ -38,19 +53,26 @@ if [ -n "$GO_FILES" ]; then fi # Check JSON files -JSON_FILES=$(echo "$STAGED_FILES" | grep '\.json$' || true) +JSON_FILES="$(printf '%s\n' "$STAGED_FILES" | grep '\.json$' || true)" if [ -n "$JSON_FILES" ]; then echo "Validating JSON files..." - for file in $JSON_FILES; do + if ! command -v jq &> /dev/null; then + echo "❌ jq not found. Please install jq to validate JSON files." + exit 1 + fi + while IFS= read -r file; do + if [ -z "$file" ]; then + continue + fi if ! jq empty "$file" 2>/dev/null; then echo "❌ Invalid JSON: $file" exit 1 fi - done + done <<< "$JSON_FILES" fi # Check Markdown files -MD_FILES=$(echo "$STAGED_FILES" | grep '\.md$' || true) +MD_FILES="$(printf '%s\n' "$STAGED_FILES" | grep '\.md$' || true)" if [ -n "$MD_FILES" ]; then echo "Checking markdown files..." # Add markdown linting if needed diff --git a/install.bat b/install.bat index 0fe6f0c..298a940 100644 --- a/install.bat +++ b/install.bat @@ -46,17 +46,23 @@ echo. echo codeagent-wrapper installed successfully at: echo %DEST% -rem Automatically ensure %USERPROFILE%\bin is in the USER (HKCU) PATH +rem Ensure %USERPROFILE%\bin is in PATH without duplicating entries rem 1) Read current user PATH from registry (REG_SZ or REG_EXPAND_SZ) set "USER_PATH_RAW=" -set "USER_PATH_TYPE=" for /f "tokens=1,2,*" %%A in ('reg query "HKCU\Environment" /v Path 2^>nul ^| findstr /I /R "^ *Path *REG_"') do ( - set "USER_PATH_TYPE=%%B" set "USER_PATH_RAW=%%C" ) rem Trim leading spaces from USER_PATH_RAW for /f "tokens=* delims= " %%D in ("!USER_PATH_RAW!") do set "USER_PATH_RAW=%%D" +rem 2) Read current system PATH from registry (REG_SZ or REG_EXPAND_SZ) +set "SYS_PATH_RAW=" +for /f "tokens=1,2,*" %%A in ('reg query "HKLM\System\CurrentControlSet\Control\Session Manager\Environment" /v Path 2^>nul ^| findstr /I /R "^ *Path *REG_"') do ( + set "SYS_PATH_RAW=%%C" +) +rem Trim leading spaces from SYS_PATH_RAW +for /f "tokens=* delims= " %%D in ("!SYS_PATH_RAW!") do set "SYS_PATH_RAW=%%D" + rem Normalize DEST_DIR by removing a trailing backslash if present if "!DEST_DIR:~-1!"=="\" set "DEST_DIR=!DEST_DIR:~0,-1!" @@ -67,42 +73,63 @@ set "SEARCH_EXP2=;!DEST_DIR!\;" set "SEARCH_LIT=;!PCT!USERPROFILE!PCT!\bin;" set "SEARCH_LIT2=;!PCT!USERPROFILE!PCT!\bin\;" -rem Prepare user PATH variants for containment tests -set "CHECK_RAW=;!USER_PATH_RAW!;" -set "USER_PATH_EXP=!USER_PATH_RAW!" -if defined USER_PATH_EXP call set "USER_PATH_EXP=%%USER_PATH_EXP%%" -set "CHECK_EXP=;!USER_PATH_EXP!;" +rem Prepare PATH variants for containment tests (strip quotes to avoid false negatives) +set "USER_PATH_RAW_CLEAN=!USER_PATH_RAW:"=!" +set "SYS_PATH_RAW_CLEAN=!SYS_PATH_RAW:"=!" -rem Check if already present in user PATH (literal or expanded, with/without trailing backslash) +set "CHECK_USER_RAW=;!USER_PATH_RAW_CLEAN!;" +set "USER_PATH_EXP=!USER_PATH_RAW_CLEAN!" +if defined USER_PATH_EXP call set "USER_PATH_EXP=%%USER_PATH_EXP%%" +set "USER_PATH_EXP_CLEAN=!USER_PATH_EXP:"=!" +set "CHECK_USER_EXP=;!USER_PATH_EXP_CLEAN!;" + +set "CHECK_SYS_RAW=;!SYS_PATH_RAW_CLEAN!;" +set "SYS_PATH_EXP=!SYS_PATH_RAW_CLEAN!" +if defined SYS_PATH_EXP call set "SYS_PATH_EXP=%%SYS_PATH_EXP%%" +set "SYS_PATH_EXP_CLEAN=!SYS_PATH_EXP:"=!" +set "CHECK_SYS_EXP=;!SYS_PATH_EXP_CLEAN!;" + +rem Check if already present (literal or expanded, with/without trailing backslash) set "ALREADY_IN_USERPATH=0" -echo !CHECK_RAW! | findstr /I /C:"!SEARCH_LIT!" /C:"!SEARCH_LIT2!" >nul && set "ALREADY_IN_USERPATH=1" +echo(!CHECK_USER_RAW! | findstr /I /C:"!SEARCH_LIT!" /C:"!SEARCH_LIT2!" >nul && set "ALREADY_IN_USERPATH=1" if "!ALREADY_IN_USERPATH!"=="0" ( - echo !CHECK_EXP! | findstr /I /C:"!SEARCH_EXP!" /C:"!SEARCH_EXP2!" >nul && set "ALREADY_IN_USERPATH=1" + echo(!CHECK_USER_EXP! | findstr /I /C:"!SEARCH_EXP!" /C:"!SEARCH_EXP2!" >nul && set "ALREADY_IN_USERPATH=1" +) + +set "ALREADY_IN_SYSPATH=0" +echo(!CHECK_SYS_RAW! | findstr /I /C:"!SEARCH_LIT!" /C:"!SEARCH_LIT2!" >nul && set "ALREADY_IN_SYSPATH=1" +if "!ALREADY_IN_SYSPATH!"=="0" ( + echo(!CHECK_SYS_EXP! | findstr /I /C:"!SEARCH_EXP!" /C:"!SEARCH_EXP2!" >nul && set "ALREADY_IN_SYSPATH=1" ) if "!ALREADY_IN_USERPATH!"=="1" ( echo User PATH already includes %%USERPROFILE%%\bin. ) else ( - rem Not present: append to user PATH using setx without duplicating system PATH - if defined USER_PATH_RAW ( - set "USER_PATH_NEW=!USER_PATH_RAW!" - if not "!USER_PATH_NEW:~-1!"==";" set "USER_PATH_NEW=!USER_PATH_NEW!;" - set "USER_PATH_NEW=!USER_PATH_NEW!!PCT!USERPROFILE!PCT!\bin" + if "!ALREADY_IN_SYSPATH!"=="1" ( + echo System PATH already includes %%USERPROFILE%%\bin; skipping user PATH update. ) else ( - set "USER_PATH_NEW=!PCT!USERPROFILE!PCT!\bin" - ) - rem Persist update to HKCU\Environment\Path (user scope) - setx PATH "!USER_PATH_NEW!" >nul - if errorlevel 1 ( - echo WARNING: Failed to append %%USERPROFILE%%\bin to your user PATH. - ) else ( - echo Added %%USERPROFILE%%\bin to your user PATH. + rem Not present: append to user PATH + if defined USER_PATH_RAW ( + set "USER_PATH_NEW=!USER_PATH_RAW!" + if not "!USER_PATH_NEW:~-1!"==";" set "USER_PATH_NEW=!USER_PATH_NEW!;" + set "USER_PATH_NEW=!USER_PATH_NEW!!PCT!USERPROFILE!PCT!\bin" + ) else ( + set "USER_PATH_NEW=!PCT!USERPROFILE!PCT!\bin" + ) + rem Persist update to HKCU\Environment\Path (user scope) + setx Path "!USER_PATH_NEW!" >nul + if errorlevel 1 ( + echo WARNING: Failed to append %%USERPROFILE%%\bin to your user PATH. + ) else ( + echo Added %%USERPROFILE%%\bin to your user PATH. + ) ) ) -rem Update current session PATH so codex-wrapper is immediately available +rem Update current session PATH so codeagent-wrapper is immediately available set "CURPATH=;%PATH%;" -echo !CURPATH! | findstr /I /C:"!SEARCH_EXP!" /C:"!SEARCH_EXP2!" /C:"!SEARCH_LIT!" /C:"!SEARCH_LIT2!" >nul +set "CURPATH_CLEAN=!CURPATH:"=!" +echo(!CURPATH_CLEAN! | findstr /I /C:"!SEARCH_EXP!" /C:"!SEARCH_EXP2!" /C:"!SEARCH_LIT!" /C:"!SEARCH_LIT2!" >nul if errorlevel 1 set "PATH=!DEST_DIR!;!PATH!" goto :cleanup diff --git a/install.py b/install.py index b886500..44cbe95 100644 --- a/install.py +++ b/install.py @@ -17,7 +17,10 @@ from datetime import datetime from pathlib import Path from typing import Any, Dict, Iterable, List, Optional -import jsonschema +try: + import jsonschema +except ImportError: # pragma: no cover + jsonschema = None DEFAULT_INSTALL_DIR = "~/.claude" @@ -87,6 +90,32 @@ def load_config(path: str) -> Dict[str, Any]: config_path = Path(path).expanduser().resolve() config = _load_json(config_path) + if jsonschema is None: + print( + "WARNING: python package 'jsonschema' is not installed; " + "skipping config validation. To enable validation run:\n" + " python3 -m pip install jsonschema\n", + file=sys.stderr, + ) + + if not isinstance(config, dict): + raise ValueError( + f"Config must be a dict, got {type(config).__name__}. " + "Check your config.json syntax." + ) + + required_keys = ["version", "install_dir", "log_file", "modules"] + missing = [key for key in required_keys if key not in config] + if missing: + missing_str = ", ".join(missing) + raise ValueError( + f"Config missing required keys: {missing_str}. " + "Install jsonschema for better validation: " + "python3 -m pip install jsonschema" + ) + + return config + schema_candidates = [ config_path.parent / "config.schema.json", Path(__file__).resolve().with_name("config.schema.json"), @@ -357,26 +386,47 @@ def op_run_command(op: Dict[str, Any], ctx: Dict[str, Any]) -> None: stderr_lines: List[str] = [] # Read stdout and stderr in real-time - import selectors - sel = selectors.DefaultSelector() - sel.register(process.stdout, selectors.EVENT_READ) # type: ignore[arg-type] - sel.register(process.stderr, selectors.EVENT_READ) # type: ignore[arg-type] + if sys.platform == "win32": + # On Windows, use threads instead of selectors (pipes aren't selectable) + import threading - while process.poll() is None or sel.get_map(): - for key, _ in sel.select(timeout=0.1): - line = key.fileobj.readline() # type: ignore[union-attr] - if not line: - sel.unregister(key.fileobj) - continue - if key.fileobj == process.stdout: - stdout_lines.append(line) - print(line, end="", flush=True) - else: - stderr_lines.append(line) - print(line, end="", file=sys.stderr, flush=True) + def read_output(pipe, lines, file=None): + for line in iter(pipe.readline, ''): + lines.append(line) + print(line, end="", flush=True, file=file) + pipe.close() - sel.close() - process.wait() + stdout_thread = threading.Thread(target=read_output, args=(process.stdout, stdout_lines)) + stderr_thread = threading.Thread(target=read_output, args=(process.stderr, stderr_lines, sys.stderr)) + + stdout_thread.start() + stderr_thread.start() + + stdout_thread.join() + stderr_thread.join() + process.wait() + else: + # On Unix, use selectors for more efficient I/O + import selectors + sel = selectors.DefaultSelector() + sel.register(process.stdout, selectors.EVENT_READ) # type: ignore[arg-type] + sel.register(process.stderr, selectors.EVENT_READ) # type: ignore[arg-type] + + while process.poll() is None or sel.get_map(): + for key, _ in sel.select(timeout=0.1): + line = key.fileobj.readline() # type: ignore[union-attr] + if not line: + sel.unregister(key.fileobj) + continue + if key.fileobj == process.stdout: + stdout_lines.append(line) + print(line, end="", flush=True) + else: + stderr_lines.append(line) + print(line, end="", file=sys.stderr, flush=True) + + sel.close() + process.wait() write_log( { diff --git a/install.sh b/install.sh index a90f87a..d9e2eec 100644 --- a/install.sh +++ b/install.sh @@ -1,12 +1,15 @@ #!/bin/bash set -e -echo "⚠️ WARNING: install.sh is LEGACY and will be removed in future versions." -echo "Please use the new installation method:" -echo " python3 install.py --install-dir ~/.claude" -echo "" -echo "Continuing with legacy installation in 5 seconds..." -sleep 5 +if [ -z "${SKIP_WARNING:-}" ]; then + echo "⚠️ WARNING: install.sh is LEGACY and will be removed in future versions." + echo "Please use the new installation method:" + echo " python3 install.py --install-dir ~/.claude" + echo "" + echo "Set SKIP_WARNING=1 to bypass this message" + echo "Continuing with legacy installation in 5 seconds..." + sleep 5 +fi # Detect platform OS=$(uname -s | tr '[:upper:]' '[:lower:]') @@ -31,23 +34,42 @@ if ! curl -fsSL "$URL" -o /tmp/codeagent-wrapper; then exit 1 fi -mkdir -p "$HOME/bin" +INSTALL_DIR="${INSTALL_DIR:-$HOME/.claude}" +BIN_DIR="${INSTALL_DIR}/bin" +mkdir -p "$BIN_DIR" -mv /tmp/codeagent-wrapper "$HOME/bin/codeagent-wrapper" -chmod +x "$HOME/bin/codeagent-wrapper" +mv /tmp/codeagent-wrapper "${BIN_DIR}/codeagent-wrapper" +chmod +x "${BIN_DIR}/codeagent-wrapper" -if "$HOME/bin/codeagent-wrapper" --version >/dev/null 2>&1; then - echo "codeagent-wrapper installed successfully to ~/bin/codeagent-wrapper" +if "${BIN_DIR}/codeagent-wrapper" --version >/dev/null 2>&1; then + echo "codeagent-wrapper installed successfully to ${BIN_DIR}/codeagent-wrapper" else echo "ERROR: installation verification failed" >&2 exit 1 fi -if [[ ":$PATH:" != *":$HOME/bin:"* ]]; then +# Auto-add to shell config files with idempotency +if [[ ":${PATH}:" != *":${BIN_DIR}:"* ]]; then echo "" - echo "WARNING: ~/bin is not in your PATH" - echo "Add this line to your ~/.bashrc or ~/.zshrc:" - echo "" - echo " export PATH=\"\$HOME/bin:\$PATH\"" + echo "WARNING: ${BIN_DIR} is not in your PATH" + + # Detect shell config file + if [ -n "$ZSH_VERSION" ]; then + RC_FILE="$HOME/.zshrc" + else + RC_FILE="$HOME/.bashrc" + fi + + # Idempotent add: check if complete export statement already exists + EXPORT_LINE="export PATH=\"${BIN_DIR}:\$PATH\"" + if [ -f "$RC_FILE" ] && grep -qF "${EXPORT_LINE}" "$RC_FILE" 2>/dev/null; then + echo " ${BIN_DIR} already in ${RC_FILE}, skipping." + else + echo " Adding to ${RC_FILE}..." + echo "" >> "$RC_FILE" + echo "# Added by myclaude installer" >> "$RC_FILE" + echo "export PATH=\"${BIN_DIR}:\$PATH\"" >> "$RC_FILE" + echo " Done. Run 'source ${RC_FILE}' or restart shell." + fi echo "" fi diff --git a/requirements-driven-workflow/agents/requirements-code.md b/requirements-driven-workflow/agents/requirements-code.md index 0881f65..2a4531d 100644 --- a/requirements-driven-workflow/agents/requirements-code.md +++ b/requirements-driven-workflow/agents/requirements-code.md @@ -104,6 +104,10 @@ You adhere to core software engineering principles like KISS (Keep It Simple, St ## Implementation Constraints +### Language Rules +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, SQL, CRUD, etc.) in English; translate explanatory text only. + ### MUST Requirements - **Working Solution**: Code must fully implement the specified functionality - **Integration Compatibility**: Must work seamlessly with existing codebase diff --git a/requirements-driven-workflow/agents/requirements-generate.md b/requirements-driven-workflow/agents/requirements-generate.md index b60fee3..272525e 100644 --- a/requirements-driven-workflow/agents/requirements-generate.md +++ b/requirements-driven-workflow/agents/requirements-generate.md @@ -88,6 +88,10 @@ Each phase should be independently deployable and testable. ## Key Constraints +### Language Rules +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, SQL, CRUD, etc.) in English; translate explanatory text only. + ### MUST Requirements - **Direct Implementability**: Every item must be directly translatable to code - **Specific Technical Details**: Include exact file paths, function names, table schemas diff --git a/requirements-driven-workflow/agents/requirements-review.md b/requirements-driven-workflow/agents/requirements-review.md index 85a604f..5fa62f3 100644 --- a/requirements-driven-workflow/agents/requirements-review.md +++ b/requirements-driven-workflow/agents/requirements-review.md @@ -176,6 +176,10 @@ You adhere to core software engineering principles like KISS (Keep It Simple, St ## Key Constraints +### Language Rules +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, E2E, CI/CD, etc.) in English; translate explanatory text only. + ### MUST Requirements - **Functional Verification**: Verify all specified functionality works - **Integration Testing**: Ensure seamless integration with existing code diff --git a/requirements-driven-workflow/agents/requirements-testing.md b/requirements-driven-workflow/agents/requirements-testing.md index ef07905..00c83af 100644 --- a/requirements-driven-workflow/agents/requirements-testing.md +++ b/requirements-driven-workflow/agents/requirements-testing.md @@ -199,6 +199,10 @@ func TestAPIEndpoint(t *testing.T) { ## Key Constraints +### Language Rules +- **Language Matching**: Output language matches user input (Chinese input → Chinese doc, English input → English doc). When language is ambiguous, default to Chinese. +- **Technical Terms**: Keep technical terms (API, E2E, CI/CD, Mock, etc.) in English; translate explanatory text only. + ### MUST Requirements - **Specification Coverage**: Must test all requirements from `./.claude/specs/{feature_name}/requirements-spec.md` - **Critical Path Testing**: Must test all critical business functionality diff --git a/skills/codeagent/SKILL.md b/skills/codeagent/SKILL.md index bd00552..7e197eb 100644 --- a/skills/codeagent/SKILL.md +++ b/skills/codeagent/SKILL.md @@ -39,18 +39,42 @@ codeagent-wrapper --backend gemini "simple task" ## Backends -| Backend | Command | Description | -|---------|---------|-------------| -| codex | `--backend codex` | OpenAI Codex (default) | -| claude | `--backend claude` | Anthropic Claude | -| gemini | `--backend gemini` | Google Gemini | +| Backend | Command | Description | Best For | +|---------|---------|-------------|----------| +| codex | `--backend codex` | OpenAI Codex (default) | Code analysis, complex development | +| claude | `--backend claude` | Anthropic Claude | Simple tasks, documentation, prompts | +| gemini | `--backend gemini` | Google Gemini | UI/UX prototyping | + +### Backend Selection Guide + +**Codex** (default): +- Deep code understanding and complex logic implementation +- Large-scale refactoring with precise dependency tracking +- Algorithm optimization and performance tuning +- Example: "Analyze the call graph of @src/core and refactor the module dependency structure" + +**Claude**: +- Quick feature implementation with clear requirements +- Technical documentation, API specs, README generation +- Professional prompt engineering (e.g., product requirements, design specs) +- Example: "Generate a comprehensive README for @package.json with installation, usage, and API docs" + +**Gemini**: +- UI component scaffolding and layout prototyping +- Design system implementation with style consistency +- Interactive element generation with accessibility support +- Example: "Create a responsive dashboard layout with sidebar navigation and data visualization cards" + +**Backend Switching**: +- Start with Codex for analysis, switch to Claude for documentation, then Gemini for UI implementation +- Use per-task backend selection in parallel mode to optimize for each task's strengths ## Parameters - `task` (required): Task description, supports `@file` references - `working_dir` (optional): Working directory (default: current) - `--backend` (optional): Select AI backend (codex/claude/gemini, default: codex) - - **Note**: Claude backend defaults to `--dangerously-skip-permissions` for automation compatibility + - **Note**: Claude backend only adds `--dangerously-skip-permissions` when explicitly enabled ## Return Format @@ -77,11 +101,12 @@ EOF ## Parallel Execution -**With global backend**: +**Default (summary mode - context-efficient):** ```bash -codeagent-wrapper --parallel --backend claude <<'EOF' +codeagent-wrapper --parallel <<'EOF' ---TASK--- id: task1 +backend: codex workdir: /path/to/dir ---CONTENT--- task content @@ -93,6 +118,17 @@ dependent task EOF ``` +**Full output mode (for debugging):** +```bash +codeagent-wrapper --parallel --full-output <<'EOF' +... +EOF +``` + +**Output Modes:** +- **Summary (default)**: Structured report with changes, output, verification, and review summary. +- **Full (`--full-output`)**: Complete task messages. Use only when debugging specific failures. + **With per-task backend**: ```bash codeagent-wrapper --parallel <<'EOF' @@ -123,9 +159,9 @@ Set `CODEAGENT_MAX_PARALLEL_WORKERS` to limit concurrent tasks (default: unlimit ## Environment Variables - `CODEX_TIMEOUT`: Override timeout in milliseconds (default: 7200000 = 2 hours) -- `CODEAGENT_SKIP_PERMISSIONS`: Control permission checks - - For **Claude** backend: Set to `true`/`1` to **disable** `--dangerously-skip-permissions` (default: enabled) - - For **Codex/Gemini** backends: Set to `true`/`1` to enable permission skipping (default: disabled) +- `CODEAGENT_SKIP_PERMISSIONS`: Control Claude CLI permission checks + - For **Claude** backend: Set to `true`/`1` to add `--dangerously-skip-permissions` (default: disabled) + - For **Codex/Gemini** backends: Currently has no effect - `CODEAGENT_MAX_PARALLEL_WORKERS`: Limit concurrent tasks in parallel mode (default: unlimited, recommended: 8) ## Invocation Pattern @@ -158,9 +194,8 @@ Bash tool parameters: ## Security Best Practices -- **Claude Backend**: Defaults to `--dangerously-skip-permissions` for automation workflows - - To enforce permission checks with Claude: Set `CODEAGENT_SKIP_PERMISSIONS=true` -- **Codex/Gemini Backends**: Permission checks enabled by default +- **Claude Backend**: Permission checks enabled by default + - To skip checks: set `CODEAGENT_SKIP_PERMISSIONS=true` or pass `--skip-permissions` - **Concurrency Limits**: Set `CODEAGENT_MAX_PARALLEL_WORKERS` in production to prevent resource exhaustion - **Automation Context**: This wrapper is designed for AI-driven automation where permission prompts would block execution