Skip to content

feat(hooks): add agent turn end hook#89152

Open
mantisai-bot wants to merge 10 commits into
openclaw:mainfrom
mantisai-bot:feat/agent-turn-end-hook-clean
Open

feat(hooks): add agent turn end hook#89152
mantisai-bot wants to merge 10 commits into
openclaw:mainfrom
mantisai-bot:feat/agent-turn-end-hook-clean

Conversation

@mantisai-bot

@mantisai-bot mantisai-bot commented Jun 1, 2026

Copy link
Copy Markdown

Summary

Adds a clean agent:turn:end internal hook event for ACP runtime turn completion. This split intentionally keeps the event as a runtime lifecycle boundary only; it does not claim transcript/session-write durability.

Changes

  • Registers and validates agent:turn:end hook events with sessionKey, success, durationMs, and optional errorCode context.
  • Emits the event from the ACP manager after terminal runtime turn handling and awaited ACP event-delivery callbacks.
  • Does not emit agent:turn:end for pre-runtime session initialization failures such as max-concurrency rejection.
  • Documents the lifecycle-only contract and explicitly states that this event does not prove durable session writes.
  • Adds focused ACP manager and internal hook tests for success, failure, ordering, and pre-runtime init-failure gating.

Real behavior proof

  • Behavior or issue addressed: proves the clean split branch emits agent:turn:end from an actual ACP runtime turn, while keeping save/write-durability concerns out of this PR and avoiding turn-end emission for pre-runtime init failures.
  • Real environment tested: local OpenClaw source checkout on macOS at /Users/mantis/Projects/openclaw-pr-turn-end-clean, branch feat/agent-turn-end-hook-clean, rebased onto current origin/main, head 52d28708c0.
  • Exact steps or command run after this patch: started the source-built Gateway from this branch with a temporary config/state directory, enabled the embedded acpx runtime plugin, loaded a temporary internal hook through hooks.internal.load.extraDirs, initialized a real ACPX/Codex persistent session using a temporary copied CODEX_HOME, ran AcpSessionManager.runTurn() with the prompt Reply with exactly TURN-END-HOOK-PROOF-OK, then attempted a second runtime session while acp.maxConcurrentSessions was 1 to force a pre-runtime ACP_SESSION_INIT_FAILED path.
  • Evidence after fix (terminal capture):
$ git rev-parse --short=10 HEAD
52d28708c0
$ OPENCLAW_LIVE_TEST=1 OPENCLAW_LIVE_ACP_BIND_CODEX_MODEL=gpt-5.5 \
  OPENCLAW_TURN_END_SOURCE_CODEX_HOME=/Users/mantis/.codex \
  OPENCLAW_TURN_END_PROOF_HEAD=$(git rev-parse --short=10 HEAD) \
  node --import tsx .artifacts/turn-end-proof-run.mjs
{
  "ok": true,
  "head": "52d28708c0",
  "initializedBackend": "acpx",
  "successSessionKey": "agent:proof-codex:acp:turn-end-proof-f0e595eb-8a63-42f0-80aa-b6e373699f0c",
  "blockedSessionKey": "agent:proof-codex:acp:turn-end-blocked-4cdae395-c82e-4e0f-b466-ef6a1e0f1486",
  "successHookContext": {
    "sessionKey": "agent:proof-codex:acp:turn-end-proof-f0e595eb-8a63-42f0-80aa-b6e373699f0c",
    "success": true,
    "durationMs": 4200
  },
  "blockedError": {
    "name": "AcpRuntimeError",
    "code": "ACP_SESSION_INIT_FAILED",
    "message": "ACP max concurrent sessions reached (1/1)."
  },
  "hookEventCount": 1,
  "blockedHookEventCount": 0,
  "proofLog": "/Users/mantis/.openclaw/state/tmp/openclaw-turn-end-proof-wuPoN6/turn-end-proof.jsonl"
}
  • Additional focused validation:
$ node scripts/run-vitest.mjs run src/acp/control-plane/manager.test.ts src/hooks/internal-hooks.test.ts
[test] passed 2 Vitest shards in 6.99s
$ pnpm check:test-types
<exit 0>
$ pnpm exec oxlint src/acp/control-plane/manager.core.ts src/acp/control-plane/manager.turn-runner.ts src/acp/control-plane/manager.test-helpers.ts src/acp/control-plane/manager.test.ts src/acp/control-plane/manager.types.ts src/hooks/internal-hooks.ts src/hooks/internal-hooks.test.ts
<exit 0>
$ pnpm exec oxfmt --check --threads=1 docs/automation/hooks.md src/acp/control-plane/manager.core.ts src/acp/control-plane/manager.turn-runner.ts src/acp/control-plane/manager.test-helpers.ts src/acp/control-plane/manager.test.ts src/acp/control-plane/manager.types.ts src/hooks/internal-hooks.ts src/hooks/internal-hooks.test.ts
All matched files use the correct format.
$ git diff --check origin/main...HEAD && git diff --check
<exit 0>
  • Observed result after fix: the temporary hook captured exactly one real agent:turn:end event from a live ACPX/Codex turn with success: true; the pre-runtime max-concurrency failure returned ACP_SESSION_INIT_FAILED and captured zero agent:turn:end events for that blocked session.
  • What was not tested: production gateway deployment. This was intentionally run against a temporary local Gateway/config/state using source code from this branch.
  • Proof limitations or environment constraints: the proof uses local Codex auth copied into a temporary Codex home and a one-off local proof harness that is not committed; hook handlers are internal and only observable when an operator installs/enables a hook.

Testing

  • Real source-built ACPX/Codex turn proof described above
  • node scripts/run-vitest.mjs run src/acp/control-plane/manager.test.ts src/hooks/internal-hooks.test.ts
  • pnpm check:test-types
  • pnpm exec oxlint src/acp/control-plane/manager.core.ts src/acp/control-plane/manager.turn-runner.ts src/acp/control-plane/manager.test-helpers.ts src/acp/control-plane/manager.test.ts src/acp/control-plane/manager.types.ts src/hooks/internal-hooks.ts src/hooks/internal-hooks.test.ts
  • pnpm exec oxfmt --check --threads=1 docs/automation/hooks.md src/acp/control-plane/manager.core.ts src/acp/control-plane/manager.turn-runner.ts src/acp/control-plane/manager.test-helpers.ts src/acp/control-plane/manager.test.ts src/acp/control-plane/manager.types.ts src/hooks/internal-hooks.ts src/hooks/internal-hooks.test.ts
  • git diff --check origin/main...HEAD && git diff --check

Related

Related #37833
Split from #71399
Sibling save-write outcome work: #89154

Latest head refresh (2026-06-22)

Current head: 99c51ee900f6e1af2a812ebf54fbf551689c3769

Merged current origin/main (3ff59df960) into the PR branch, resolved Plugin SDK surface report/test conflicts against the merged hook surface, and aligned the slash-command browser-import expectation with the current shared-registry import order.

Validation on the current head:

node scripts/run-vitest.mjs run src/acp/control-plane/manager.test.ts src/hooks/internal-hooks.test.ts
pnpm exec vitest run ui/src/ui/chat/slash-commands.browser-import.test.ts test/scripts/plugin-sdk-surface-report.test.ts

Observed results:

  • Focused ACP manager + internal-hook tests passed 2 shards / 75 tests.
  • Focused slash-command import + Plugin SDK surface tests passed 2 files / 7 tests.

Limitations: I attempted to rerun the existing live ACPX/Codex agent:turn:end proof harness on this head, but the temporary Gateway run failed with ACP_TURN_FAILED / Internal error before producing hook evidence. I am not claiming refreshed live ACP proof for this head; the prior live proof remains from the earlier proof head.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 9:03 AM ET / 13:03 UTC.

Summary
The PR adds a documented internal ACP agent:turn:end hook event with typed context, ACP manager dispatch, validation/tests, and broad merge-refresh formatting updates across adjacent files.

Reproducibility: not applicable. this is a feature/API PR, not a bug report. Source inspection shows current main lacks the hook, while the PR body demonstrates the proposed behavior only on an older head.

Review metrics: 2 noteworthy metrics.

  • Internal hook API: 1 documented event added. The event name, timing, and context fields become consumer-visible once released.
  • Plugin SDK surface budget: public exports 10327→10330; callable exports 5184→5185. The PR updates the public surface budget, so maintainers should explicitly accept the contract growth before merge.

Stored data model
Persistent data-model change detected: unknown-truncated-pull-files, vector/embedding metadata: extensions/memory-wiki/src/okf.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #89152
Summary: This PR is the current split candidate for the ACP turn-end lifecycle hook; the older umbrella PR was closed unmerged and the sibling PR covers transcript-save durability.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase or otherwise refresh the branch to a clean merge state.
  • Rerun current-head real ACPX/Codex agent:turn:end proof with redacted terminal/log evidence.
  • Get maintainer acceptance of the event name, timing, payload, and lifecycle-only boundary.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Earlier terminal proof shows the behavior on old head 52d28708c0, but current head 99c51ee900f6e1af2a812ebf54fbf551689c3769 has no successful refreshed real ACPX/Codex proof and reports a failed proof attempt; add redacted terminal/log proof and update the PR body.

Risk before merge

  • [P1] The new agent:turn:end event name, timing, and payload become a documented hook contract that consumers may depend on once released.
  • [P1] The latest PR head lacks successful refreshed real ACPX/Codex proof; the PR body explicitly says the current-head proof attempt failed before hook evidence was produced.
  • [P1] GitHub currently reports the branch as dirty/unmergeable, so maintainers cannot review the final three-way merge result yet.
  • [P1] Maintainers still need to accept the lifecycle-only boundary, especially that it is separate from durable transcript/session-write semantics and the sibling save-hook PR.

Maintainer options:

  1. Refresh proof and sponsor the contract (recommended)
    Require a clean current-head rebase, successful real ACPX/Codex hook proof, and maintainer acceptance of the event name, timing, and payload before merge.
  2. Accept prior proof deliberately
    Maintainers may choose to accept the older live proof plus focused tests if they decide the merge-refresh changes did not affect the runtime path.
  3. Pause the API surface
    If maintainers do not want this documented internal hook API now, pause or close the PR for product direction instead of landing it by default.

Next step before merge

  • [P1] Maintainers need to decide whether to sponsor the new hook contract, and the contributor needs current-head proof plus a clean branch; there is no narrow automated code repair to queue.

Security
Cleared: No concrete security or supply-chain concern was found in the hook dispatch, docs/tests, formatting-only script changes, or absence of dependency/workflow changes.

Review details

Best possible solution:

Land only after maintainers sponsor the hook contract, the branch is clean, and current-head real ACPX/Codex proof verifies the lifecycle-only, non-durable semantics.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a feature/API PR, not a bug report. Source inspection shows current main lacks the hook, while the PR body demonstrates the proposed behavior only on an older head.

Is this the best way to solve the issue?

Unclear until maintainer API acceptance: the implementation uses the existing internal hook dispatcher and separates transcript durability into the sibling save-hook work, but current-head proof, clean mergeability, and contract sponsorship remain open.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 153fed790a5c.

Label changes

Label justifications:

  • P2: This is a normal-priority internal hook/API feature with limited runtime blast radius but real maintainer contract-review impact.
  • merge-risk: 🚨 compatibility: Merging creates a new documented hook contract whose payload and lifecycle timing may need to remain stable for hook consumers.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • feature: ✨ showcase: ClawSweeper spotlight: unusually compelling feature idea for maintainer attention. A deterministic ACP turn lifecycle hook is a compelling workflow unlock for post-turn memory, analytics, and orchestration when kept separate from transcript durability.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Earlier terminal proof shows the behavior on old head 52d28708c0, but current head 99c51ee900f6e1af2a812ebf54fbf551689c3769 has no successful refreshed real ACPX/Codex proof and reports a failed proof attempt; add redacted terminal/log proof and update the PR body.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Recent live GitHub history shows ACP turn-runner extraction, session-flow refactors, and hook documentation work on the central files this PR changes. (role: recent ACP manager and hook docs contributor; confidence: high; commits: faae7529fde0, 118b9cacf677, 05289f1aa045; files: src/acp/control-plane/manager.core.ts, src/acp/control-plane/manager.turn-runner.ts, docs/automation/hooks.md)
  • vincentkoc: Recent commits touched ACP error normalization, ACP helper types, and hook/type seam extraction near the manager and internal hook surfaces. (role: recent adjacent ACP/hooks contributor; confidence: medium; commits: b10fedb7de21, f9439715e99b, 97d1b88e3faa; files: src/acp/control-plane/manager.core.ts, src/acp/control-plane/manager.types.ts, src/hooks/internal-hooks.ts)
  • osolmaz: Earlier ACP thread-bound manager/control-plane work introduced much of the ACP lifecycle area that this hook extends. (role: historical ACP feature-area contributor; confidence: medium; commits: a7d56e3554d0, 154a7edb7cad; files: src/acp/control-plane/manager.core.ts, src/acp/control-plane/manager.turn-runner.ts, src/acp/control-plane/manager.types.ts)
  • Drickon: History links global singleton hook registry behavior and message hook context expansion to this contributor, adjacent to the hook delivery contract expanded here. (role: adjacent hook registry contributor; confidence: medium; commits: 0d8beeb4e5f5, b5102ba4f98e, d0a3743abdd1; files: src/hooks/internal-hooks.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 1, 2026
@mantisai-bot
mantisai-bot force-pushed the feat/agent-turn-end-hook-clean branch from 9720628 to 9aecd5c Compare June 1, 2026 17:00
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 1, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 1, 2026
@mantisai-bot
mantisai-bot force-pushed the feat/agent-turn-end-hook-clean branch from 91a8d86 to 52d2870 Compare June 1, 2026 17:44
@mantisai-bot

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 4, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. feature: ✨ showcase ClawSweeper spotlight: unusually compelling feature idea for maintainer attention. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 19, 2026
…ook-clean

# Conflicts:
#	src/gateway/server-discovery-runtime.test.ts
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord channel: line Channel integration: line channel: msteams Channel integration: msteams channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: voice-call Channel integration: voice-call app: web-ui App: web-ui labels Jun 21, 2026
…ook-clean

# Conflicts:
#	extensions/slack/src/monitor/send.runtime.ts
#	src/agents/agent-tools.schema.test.ts
#	src/commands/onboard-channels.e2e.test.ts
…ook-clean

# Conflicts:
#	scripts/plugin-sdk-surface-report.mjs
#	test/scripts/plugin-sdk-surface-report.test.ts
@mantisai-bot

Copy link
Copy Markdown
Author

@osolmaz @steipete @vincentkoc @Drickon ClawSweeper is proposing close here unless a maintainer sponsors the lifecycle hook contract or marks it for human review.

From our side, we still think agent:turn:end adds meaningful functionality: it gives integrations a bounded post-runtime turn lifecycle signal without coupling them to transcript persistence. The implementation is intentionally lifecycle-only, fire-and-forget, and documented as non-durable/non-transcript.

What would you prefer we do with this PR?

  • close it as an unsponsored product/API surface
  • narrow the hook contract further
  • keep it open under maintainer sponsorship / clawsweeper:human-review

Happy to rebase and clean up current-head CI once the direction is clear; the main question seems to be product/API ownership rather than a concrete code blocker.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: feat(hooks): add agent turn end hook This is item 1/1 in the current shard. Shard 0/4.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: web-ui App: web-ui channel: discord Channel integration: discord channel: feishu Channel integration: feishu channel: irc channel: line Channel integration: line channel: mattermost Channel integration: mattermost channel: msteams Channel integration: msteams channel: qqbot channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: voice-call Channel integration: voice-call cli CLI command changes commands Command implementations docker Docker and sandbox tooling docs Improvements or additions to documentation extensions: amazon-bedrock extensions: chutes extensions: codex extensions: copilot extensions: deepinfra extensions: diffs extensions: kilocode extensions: kimi-coding extensions: memory-wiki extensions: minimax extensions: openrouter extensions: qa-lab feature: ✨ showcase ClawSweeper spotlight: unusually compelling feature idea for maintainer attention. gateway Gateway runtime merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: XL stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants