Skip to content

feat(hooks): canonical PostToolUseFailure event, mapped for Goose - #14

Merged
rsnodgrass merged 1 commit into
mainfrom
ryan/goose-posttooluse-failure
Jul 31, 2026
Merged

feat(hooks): canonical PostToolUseFailure event, mapped for Goose#14
rsnodgrass merged 1 commit into
mainfrom
ryan/goose-posttooluse-failure

Conversation

@rsnodgrass

@rsnodgrass rsnodgrass commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The gap

Goose fires PostToolUse only on success. Failures arrive on a separate PostToolUseFailure event, which nothing in agentx knew about — there was no canonical constant, and GooseAgent.EventPhases() didn't map it.

Consequence for a consumer driving session capture off hooks: a turn that ends in a failed tool call produces no event it can act on, so the failure stays invisible until the next successful tool call or Stop. On a turn that fails and then stops, that's a full turn of lost signal.

Cursor has the same split (postToolUseFailure, camelCase) and the same gap.

What this PR ships

  • HookEventPostToolUseFailure — canonical constant, PascalCase per Goose's spelling. The camelCase Cursor variant already existed as CursorEventPostToolUseFailure and is unchanged.
  • Goose maps it to PhaseAfterTool — same phase as the success path, so a consumer drains the failed turn at the same point it would have drained a successful one. EventPhases() now covers seven events.

What stays unmapped, and why

Goose fires eleven events. Four are deliberately left out:

Event Why not
BeforeReadFile strict subset of PreToolUse — reading a file is a tool call
BeforeShellExecution strict subset of PreToolUse — running a shell command is a tool call
AfterFileEdit strict subset of PostToolUse (and success-only)
AfterShellExecution strict subset of PostToolUse (and success-only)

Mapping these would fire a consumer twice per tool call for zero additional signal. A consumer that spawns a subprocess per hook would double its cost in the hot path to learn nothing new.

PhaseCompact also stays unmapped: Goose has no compaction event at all. That reasoning is now recorded on EventPhases() so the next reader doesn't have to re-derive it — or worse, "fix" it by adding the four.

Test Plan

  • make check1134 tests pass, 2 pre-existing skips, 0 lint issues
  • TestGooseEventPhases updated to seven events and still asserts no phase is PhaseCompact
  • allAgentSpecs() Goose eventPhaseCount 6 → 7

Follow-up

Cut v0.1.13 after merge. sageox/ox bumps to it as part of its Goose adapter work.

Summary by CodeRabbit

  • New Features

    • Added support for recognizing failed tool-use events as a distinct lifecycle event.
    • Goose now maps failed tool-use events to the after-tool phase.
  • Documentation

    • Added release notes explaining event mappings and why certain specialized file and shell events remain excluded.
  • Tests

    • Updated lifecycle event coverage and Goose event-phase expectations for the new failed tool-use event.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d762659e-80ae-4f37-a845-ae7d0a5dfdf8

📥 Commits

Reviewing files that changed from the base of the PR and between cacaae4 and cfc7ed3.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • agent.go
  • agents/agents_comprehensive_test.go
  • agents/goose.go
  • agents/goose_test.go

📝 Walkthrough

Walkthrough

Adds a canonical PostToolUseFailure hook event, maps it to Goose’s PhaseAfterTool, updates lifecycle expectations and tests, and documents the mapping and intentionally unmapped specialized events.

Changes

Post-tool failure event mapping

Layer / File(s) Summary
Event contract
agent.go
Adds the exported HookEventPostToolUseFailure constant for failed tool calls.
Goose phase mapping and validation
agents/goose.go, agents/goose_test.go, agents/agents_comprehensive_test.go, CHANGELOG.md
Maps the new event to PhaseAfterTool, updates expected lifecycle phase counts and mappings, and documents the release changes and unmapped specialized events.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • sageox/agentx#13: Introduced the Goose lifecycle event-phase mappings extended by this change.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ryan/goose-posttooluse-failure

Comment @coderabbitai help to get the list of available commands.

@rsnodgrass
rsnodgrass marked this pull request as ready for review July 31, 2026 00:35
@rsnodgrass
rsnodgrass merged commit 106797f into main Jul 31, 2026
5 checks passed
@rsnodgrass
rsnodgrass deleted the ryan/goose-posttooluse-failure branch July 31, 2026 00:35
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown

Greptile Summary

This change introduces a canonical failed-tool hook event, maps Goose's failure event to the after-tool lifecycle phase, and updates related coverage and documentation. A failed Cursor tool call still has no after-tool lifecycle phase: executable validation showed that both Cursor's direct map and the public registry return an empty phase for postToolUseFailure.

Confidence Score: 3/5

Not safe to merge until T-Rex findings are addressed.

T-Rex reproduced 2 failing behaviors at runtime in agents/cursor.go; the change needs fixes before it is safe to merge.

Files Needing Attention: agents/cursor.go

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced proof for a posted P1 finding; see the corresponding review comment for finding details.
  • T-Rex produced proof for another posted P1 finding; see the corresponding review comment for finding details.
  • T-Rex ran the requested contract validation check but local artifact references were not uploaded.

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. agents/cursor.go, line 173 (link)

    P1 Cursor failure event remains unmapped

    CursorEventPostToolUseFailure is publicly defined but is absent from Cursor's EventPhases() map. Consequently, a Cursor postToolUseFailure event has no PhaseAfterTool lifecycle phase in either the agent's direct mapping or the public BuildEventPhaseMap() registry output. Add agentx.CursorEventPostToolUseFailure: agentx.PhaseAfterTool alongside the successful postToolUse mapping and cover the failure event in the Cursor mapping tests.

    T-Rex Ran code and verified through T-Rex

    Fix in Claude Code

  2. General comment

    P1 Cursor omits postToolUseFailure from its lifecycle phase mapping

    • Bug
      • CursorEventPostToolUseFailure is publicly defined, but CursorAgent.EventPhases() does not associate it with PhaseAfterTool. As a result, the event is absent from the direct Cursor mapping and from BuildEventPhaseMap()["cursor"].
    • Cause
      • The EventPhaseMap literal in agents/cursor.go:169-177 includes CursorEventPostToolUse but has no CursorEventPostToolUseFailure: PhaseAfterTool entry.
    • Fix
      • Add agentx.CursorEventPostToolUseFailure: agentx.PhaseAfterTool to CursorAgent.EventPhases() and add a focused regression test covering both the direct map and BuildEventPhaseMap()["cursor"].

    T-Rex Ran code and verified through T-Rex

Fix All in Claude Code

Reviews (1): Last reviewed commit: "feat(hooks): canonical PostToolUseFailur..." | Re-trigger Greptile

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.

1 participant