feat(runtime): P6 command hook stdin/stdout JSON protocol#694
Open
Cai-Tang-www wants to merge 6 commits into
Open
feat(runtime): P6 command hook stdin/stdout JSON protocol#694Cai-Tang-www wants to merge 6 commits into
Cai-Tang-www wants to merge 6 commits into
Conversation
Implement the full command hook protocol for user/repo scopes as specified in issue 1024XEngineer#683. External commands now communicate via structured JSON on stdin/stdout instead of raw exit codes. Key changes: - New command_handler.go: protocol types (CommandHookPayload/Response), BuildCommandPayload, ParseCommandResponse, RunCommandHook with argv/shell execution modes, env isolation (NEOCODE_HOOK_* only) - stdin: single-line JSON with payload_version, hook_id, point, metadata - stdout: single-line JSON with status, message, update_input, annotations - Graceful fallback: non-JSON stdout falls back to exit code semantics (0=pass, 1/2=block, other=failed) - argv mode (default): params.command as string array, direct exec - shell mode: params.command as string + params.shell=true - update_input wired for user_prompt_submit point - annotations collected into runtime annotation buffer - Context timeout detection prioritized over exit code mapping - 18 new unit tests, all cross-platform (Windows + Unix) Closes 1024XEngineer#683 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
…ertion Windows injects PATH at the system level even when cmd.Env is set. Split into two focused tests: one verifies NEOCODE_HOOK_* vars are injected via exec, the other verifies buildCommandEnv returns the correct variable set. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
input.Metadata["point"] was not populated by runHookPoint (it only injects run_id/session_id/runtime_run_token/phase/turn), so command hooks received an empty point in the stdin payload and NEOCODE_HOOK_POINT. Fix by passing item.Point from config directly into the handler closure at build time. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Self-review fixes for P6 command hook protocol:
1. Security: non-zero exit code now takes precedence over JSON status.
A malicious script claiming {"status":"pass"} while exiting non-zero
will be treated as block/failed based on exit code, not pass.
buildResultFromExitCode still extracts message/annotations from JSON
stdout when available, but status authority remains with exit code.
2. Added top-level run_id and session_id fields to CommandHookPayload,
populated from HookContext. External scripts can now read these
directly without digging into metadata.
3. Added tests: exit code precedence over JSON, exit code 3 with JSON
message extraction, payload run_id/session_id verification, stdin
payload round-trip with run_id/session_id.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
P0 security/correctness:
- Capture stderr separately from stdout; append to failed results for debug
- Add SystemRoot/SystemDrive/USERPROFILE to Windows hook env (TLS/NTLM)
- Limit stdout to 1 MiB via pipe+LimitReader to prevent OOM
P1 design/maintainability:
- Extract ValidateCommandParams/ParseCommandParams as shared exports in
hooks package, deduplicating 3 identical validation sites
(config/runtime_hooks.go, repo_hooks.go, user_hooks.go)
- Guard buildExecCmd against Shell+multi-args misuse (panic)
- Set result.Error for exit code 1/2 block results (consistency)
P2 test coverage:
- Add 7-subtest TestApplyCommandHookUpdateInput (caught real bug: was
replacing ALL text parts instead of first-only)
- Add TestRunCommandHookEnvIsolationNoLeak (Unix PATH/HOME/USER check)
- Add TestRunCommandHookShellModeWindows (powershell -Command)
- Add TestValidateCommandParams (9 cases covering exported API)
P3 documentation:
- Document stderr handling strategy, run_id/session_id field precedence,
update_input+block interaction, exit-code-over-JSON security rule,
stdout size limit, Windows env vars
Bug fix discovered during review:
- applyCommandHookUpdateInput used update.Text="" to skip subsequent
text parts, but still appended NewTextPart(""). Fixed with replaced flag.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add targeted tests for previously uncovered branches: - ParseCommandParams: all 9 branches ([]string, []any, string+shell, nil, empty, unsupported type, empty element, shell=false) - RunCommandHook: stdout-too-large, nonexistent binary, exit 0 empty, exit 2 block, exit 3 with stderr, block+message, failed default msg, failed custom msg, pass+annotations+update_input, stdin+metadata - buildCommandEnv: verify Windows SystemRoot/SystemDrive/USERPROFILE - buildResultFromResponse: failed status with default/custom message - buildResultFromExitCode: exit 2 sets Error, exit 3 with stderr Coverage: 90.8% -> 96.0% (command_handler.go functions all >= 75%) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
实现
kind=commandhook 的完整 stdin/stdout JSON 协议(RFC #679 P6),使任意可执行脚本可作为 hook 接入 user/repo scope。核心变更
新增
internal/runtime/hooks/command_handler.go:CommandHookPayload(stdin)、CommandHookResponse(stdout)、CommandHookSpec(执行参数)BuildCommandPayload:构造 stdin JSON,包含payload_version、hook_id、point、metadataParseCommandResponse:解析 stdout JSON,提取status/message/update_input/annotationsRunCommandHook:执行外部命令,处理 stdin 管道、stdout 解析、环境隔离、超时取消执行模式:
params.command格式["python3", "hook.py"]"python3 hook.py"+params.shell: truesh -c/powershell -Command执行单字符串
params.command不设置params.shell: true触发配置校验错误,防止 shell 注入。stdin 协议(单行 JSON):
{ "payload_version": "1", "hook_id": "my-hook", "point": "before_tool_call", "metadata": {"tool_name": "bash", "workdir": "/workspace"} }stdout 协议(单行 JSON):
{ "status": "pass", "message": "optional message", "update_input": {"text": "rewritten prompt"}, "annotations": ["note1", "note2"] }stdout 退化模式: 非 JSON stdout 退化为 exit code 语义(0=pass, 1/2=block, other=failed),兼容简单脚本。
环境隔离: 命令进程仅注入
NEOCODE_HOOK_HOOK_ID、NEOCODE_HOOK_POINT、NEOCODE_HOOK_PAYLOAD_VERSION,不继承宿主环境。update_input接入: 仅user_prompt_submit点位(CanUpdateInput=true)允许,格式{"text": "..."}替换用户输入文本。annotations接入: stdout 中的annotations数组进入 runtime annotation buffer,与message一同被记录。修改文件清单
internal/runtime/hooks/command_handler.gointernal/runtime/hooks/command_handler_test.gointernal/runtime/hooks/result.goHookResultMetadata新增Annotations、UpdateInput字段internal/runtime/user_hooks.gobuildUserCommandHookHandler,新增parseCommandHookParams,移除buildCommandHookProcessinternal/runtime/repo_hooks.govalidateRepoCommandParams,支持数组格式params.commandinternal/config/runtime_hooks.govalidateRuntimeCommandItem,argv/shell 模式校验internal/config/runtime_hooks_test.gointernal/runtime/hooks_integration.gorecordUserHookAnnotations扩展收集Metadata.Annotations;新增applyCommandHookUpdateInputinternal/runtime/run.gouser_prompt_submit点位接入update_input应用internal/runtime/repo_hooks_test.godocs/runtime-hooks-design.md向后兼容
params.command为字符串的配置需添加params.shell: true,否则校验失败(符合 issue 要求:argv 为新默认模式)CombinedOutput()→Output():stderr 不再混入 message测试覆盖
NEOCODE_HOOK_*变量,无宿主环境泄漏)sh -c/powershell -Commandupdate_input解析与应用annotations解析与收集payload_version字段稳定("1")Closes #683