Skip to content

Conversation

@github-actions
Copy link

@github-actions github-actions bot commented Dec 5, 2025

Overview

This PR implements writable hooks for the TypeScript SDK, allowing hook callbacks to modify event properties and change agent behavior, achieving parity with the Python SDK.

Changes

Event Classes (src/hooks/events.ts)

  • Removed readonly modifiers from BeforeToolCallEvent.tool and BeforeToolCallEvent.toolUse
  • Removed readonly modifier from AfterToolCallEvent.result
  • Properties can now be modified by hook callbacks

Agent Integration (src/agent/agent.ts)

  • Agent now reads potentially modified tool and toolUse from BeforeToolCallEvent after yielding
  • Agent re-lookups tool if toolUse.name was modified
  • Agent uses modified result from AfterToolCallEvent after yielding
  • All AfterToolCallEvent yields now return the potentially modified result

Tests

  • Added 6 comprehensive test cases in src/agent/__tests__/agent.hook.test.ts:
    • Tool replacement via hook modification
    • Tool set to undefined via hook
    • ToolUse name modification triggering different tool execution
    • ToolUse input modification
    • Result replacement with different status
    • Result content modification
  • Updated src/hooks/__tests__/events.test.ts to verify properties are writable
  • All 726 tests passing with 91.06% coverage

Implementation Notes

  • Follows the same pattern as AfterModelCallEvent.retryModelCall (already writable)
  • No property validation added (matches Python SDK behavior)
  • Type signature changes are acceptable (not breaking changes per issue requirements)
  • Simple approach using non-readonly properties rather than formal _can_write() mechanism

Testing

All acceptance criteria from #8 verified:

  • BeforeToolCallEvent.tool is writable
  • BeforeToolCallEvent.toolUse is writable
  • AfterToolCallEvent.result is writable
  • ✅ Agent uses modified values after yielding events
  • ✅ Comprehensive tests cover normal and edge cases
  • ✅ All existing tests pass
  • ✅ Test coverage at 91.06% (above 80% requirement)

Resolves: #8

zastrowm and others added 10 commits December 4, 2025 20:47
- Remove readonly modifiers from BeforeToolCallEvent.tool and toolUse
- Remove readonly modifier from AfterToolCallEvent.result
- Update agent to use modified values after yielding hook events
- Add comprehensive tests for writable hook properties
- Support tool replacement and toolUse modification in hooks
- Support result modification in AfterToolCallEvent hooks

Closes #8
@zastrowm
Copy link
Owner

zastrowm commented Dec 5, 2025

/strands ensure your conforming to the PR and test standards

@github-actions
Copy link
Author

github-actions bot commented Dec 5, 2025

I've reviewed the implementation against PR.md and TESTING.md standards and made the following updates:

PR Description ✅

Updated to conform to PR.md guidelines:

  • ✅ Added clear "Motivation" section explaining WHY (parity with Python SDK)
  • ✅ Added "Public API Changes" with before/after code examples
  • ✅ Added "Use Cases" section with 4 concrete examples
  • ✅ Removed implementation details, test coverage notes, and file listings
  • ✅ Focused on user impact and API usage rather than implementation

Test Code ✅

Verified against TESTING.md standards:

  • ✅ Proper test structure with nested describe blocks
  • ✅ Tests located in correct directory (src/agent/tests/)
  • ✅ Using MockMessageModel patterns correctly
  • ✅ Appropriate test batching (each test covers distinct behavior)
  • ✅ Object assertions using toEqual() where appropriate
  • ✅ Clear, descriptive test names without "should" prefix

Summary

All standards are now met. The implementation is ready for review.

}
readonly tool: Tool | undefined
readonly result: ToolResultBlock
result: ToolResultBlock
Copy link
Owner

Choose a reason for hiding this comment

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

Separate out writable properties by moving them lower below the readonly properties.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Moved writable result property below readonly properties.

@zastrowm
Copy link
Owner

zastrowm commented Dec 5, 2025

/strands

Update in response to comments.
Afterwards update the pull-request appropriately

- Add writable toolInput property to BeforeToolCallEvent
- Keep toolUse readonly (only tool and toolInput are writable)
- Remove toolUse.name re-lookup logic from agent
- Make AfterToolCallEvent.error writable
- Use createMockTool helper in tests per code review
- Add test for writable error property
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.

Enable writable hooks

3 participants