Skip to content

feat(runtime): P1 接入 internal hooks 到工具与完成前生命周期(#489)#500

Merged
phantom5099 merged 3 commits into1024XEngineer:mainfrom
Cai-Tang-www:feat/issue-489-runtime-hook-p1
Apr 28, 2026
Merged

feat(runtime): P1 接入 internal hooks 到工具与完成前生命周期(#489)#500
phantom5099 merged 3 commits into1024XEngineer:mainfrom
Cai-Tang-www:feat/issue-489-runtime-hook-p1

Conversation

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator

背景

目标与范围

本 PR 仅实现 P1 约定能力:

  1. before_tool_call:支持在工具执行前拦截(可阻断)。
  2. after_tool_result:支持在工具执行后观测(只读,不改写工具结果)。
  3. before_completion_decision:在完成判定前触发(P1 仅观测,不接管终态决策)。

明确非目标:

  • 不引入 user hooks / repo hooks。
  • 不接入 acceptance/verifier 闭环决策。
  • 不改变现有 completion gate / final acceptance 的语义。

关键实现

1) Runtime 注入 Hook Executor 与事件桥接

  • 在 runtime service 中注入 hook executor(无注册 hook 时保持零影响)。
  • 新增 hooks -> runtime 事件桥接,透传 hook_started/hook_finished/hook_failed

2) 工具前拦截:before_tool_call

  • 工具实际执行前运行 hook。
  • 若任一 hook block
    • 不调用 tool manager 执行工具;
    • 生成结构化 ToolResult 错误;
    • ToolResult.ErrorClass = "hook_blocked"
    • hook_blocked 事件(含 hook_id/point/tool_name/reason)。

3) 工具后观测:after_tool_result

  • 无论工具成功、失败、取消均触发 after_tool_result
  • hook 仅接收结果摘要上下文,不可改写原始 ToolResult
  • hook 内异常按 P0 failure policy 处理,不反向破坏已产出的工具结果。

4) 完成前观测:before_completion_decision

  • 在 completion/final decision 前触发。
  • 若返回 block,P1 仅记录事件与日志(enforced=false),不改变 terminal state。

5) 契约扩展

  • tools.ToolResult 新增 ErrorClass string,用于机器可判别错误分类。
  • runtime 新增 hook_blocked 事件与 payload。
  • TUI/gateway 协议增加 hook 事件解码与展示,保证可观测。

行为约束(与 #489 对齐)

  • before_tool_callblock 必须“可执行拦截”(阻断工具执行)。
  • after_tool_result 必须“只观测,不改写结果”。
  • before_completion_decision 在 P1 阶段“只观测,不接管完成判定”。
  • 无 hook 场景行为不变。

修复的接入细节问题(本 PR 内)

  1. 修正 tool_start 事件时序:
  • 仅在 before_tool_call 放行后发 tool_start
  • 被 hook 拦截的工具调用不再错误发送 tool_start
  1. 修正取消分支漏触发 after_tool_result
  • context canceled 分支也会触发 after_tool_result,保证观测闭环一致。

测试

已补齐并通过的测试覆盖:

  • before_tool_call block 时:工具不执行,返回 ErrorClass=hook_blocked
  • before_tool_call pass 时:工具正常执行。
  • after_tool_result:可收到结果摘要,且不可改写原结果。
  • before_completion_decision block:仅观测,不改变完成路径。
  • tool_start 时序与阻断场景。
  • context canceled 仍触发 after_tool_result
  • hook 生命周期事件解码与 payload 校验(TUI/gateway)。

本地执行:

  • go test ./internal/runtime/...
  • go test ./internal/tools/...
  • go test ./internal/tui/services/...

影响面与风险

  • 影响模块:internal/runtimeinternal/toolsinternal/tui/servicesinternal/tui/core/app
  • 主要风险:事件新增带来的下游兼容性;通过“未知事件容忍 + 新增解码测试”控制。
  • 安全边界:仅 internal hooks,无新增外部执行面。

回滚方案

  • 若线上出现回归,可临时移除 runtime 的 hook executor 注入或清空 registry(无 hook 回退到旧行为)。
  • 该回滚不涉及 provider 协议变更,风险可控。

后续计划

  • P2:全局 user builtin hooks(受限上下文 + handler 白名单)。
  • P3:repo hooks + trust gate。
  • P7:将 before_completion_decision 与 acceptance/verifier 闭环真正接管完成决策。

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed 4-pass review (code quality, performance, security, docs). One noteworthy issue found.

Comment thread internal/runtime/toolexec.go
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 96.86099% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/runtime/hooks_integration.go 96.66% 2 Missing and 1 partial ⚠️
internal/runtime/toolexec.go 95.83% 1 Missing and 1 partial ⚠️
internal/tui/core/app/update.go 96.36% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/code 提升覆盖率到100

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 28, 2026

已完成本轮“提升覆盖率到100”的测试补齐,并已推送到当前 PR 分支 feat/issue-489-runtime-hook-p1(commit: 9360e6a)。

本次完成

  • 补充了 runtime hooks 接入相关分支测试:
    • internal/runtime/hooks_integration_test.go
    • 覆盖 EmitHookEvent 分支、hook 上下文 envelope 分支、block reason 分支、summarizeHookResultContent 截断分支等
  • 补充了 turn_control 新增错误分类分支测试:
    • internal/runtime/turn_control_test.go
  • 补充了 TUI hook 事件处理器分支测试与注册项断言:
    • internal/tui/core/app/update_runtime_events_test.go
  • 补充了 gateway hook 生命周期 payload 解码测试:
    • internal/tui/services/gateway_stream_client_test.go

本地验证

  • go test ./internal/runtime ./internal/tui/core/app ./internal/tui/services ./internal/tools -coverprofile=/tmp/prcover3.out 通过
  • go test ./... 全量通过

说明

  • 已针对 Codecov 报告中的主要缺口(尤其 update.go 新增 hook handler 0% 覆盖)完成补测。
  • 未在本地直接调用 Codecov API,因此最终“patch coverage=100%”以 CI 上最新 Codecov 回报为准。

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)

Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
StartedAt time.Time `json:"started_at,omitempty"`
DurationMS int64 `json:"duration_ms,omitempty"`
Error string `json:"error,omitempty"`
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xgopilot 分析hook的生命周期定义放在events.go里是否合适

Copy link
Copy Markdown

@fennoai fennoai Bot Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论

放在 internal/runtime/events.go 总体是合适的,前提是你把它视为「runtime 对外事件契约层」而不是 hook 领域模型层。

理由:

  • events.go 本身就是 runtime 对外统一事件协议(RuntimeEvent + EventType + payload)定义处,hook 生命周期事件在 runtime 总线上对 TUI/gateway 可见,属于该层职责。
  • 真正的 hook 领域定义仍在 internal/runtime/hooks(如 hooks/events.gohooks/types.go),runtime 只是桥接并外发。

边界风险(建议关注)

  1. hooksruntime 存在事件字符串双份定义,漂移风险真实存在。

    • hook 侧:internal/runtime/hooks/events.gohook_started/finished/failed
    • runtime 侧:internal/runtime/events.goEventHookStarted/Finished/Failed
    • 桥接处当前是字符串直转:internal/runtime/hooks_integration.goEventType(strings.TrimSpace(string(event.Type)))
  2. EventHookBlocked 只在 runtime 层定义是合理的。
    这是 runtime 执行语义(before_tool_call 是否 enforced)产生的事件,不是 hooks executor 原生生命周期事件。

建议落地

  • 保持当前放置位置不变(不建议把 runtime 对外事件再下沉回 hooks 包)。
  • 为降低漂移,建议补一个显式映射或守卫测试:
    • 映射:HookEventStarted -> EventHookStarted 等,避免隐式字符串 cast。
    • 测试:断言 hooks 事件全集都能被 runtime 桥接成预期 EventType
  • 若你担心 events.go 体积继续增长,可仅做文件级拆分(如 events_hook.go),包内职责不变。

本次未做

  • 未改代码,仅按你的问题完成职责边界分析。

View job run

@1024XEngineer 1024XEngineer deleted a comment from Cai-Tang-www Apr 28, 2026
@1024XEngineer 1024XEngineer deleted a comment from Cai-Tang-www Apr 28, 2026
@1024XEngineer 1024XEngineer deleted a comment from Cai-Tang-www Apr 28, 2026
@1024XEngineer 1024XEngineer deleted a comment from fennoai Bot Apr 28, 2026
@phantom5099 phantom5099 merged commit 2521416 into 1024XEngineer:main Apr 28, 2026
3 checks passed
@Cai-Tang-www Cai-Tang-www deleted the feat/issue-489-runtime-hook-p1 branch April 28, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants