Skip to content

feat: Assert hook execution order for evaluation and track stages#343

Merged
keelerm84 merged 1 commit into
feat/fdv2from
mk/NOTICKET/fix-hook-test-fdv2
May 15, 2026
Merged

feat: Assert hook execution order for evaluation and track stages#343
keelerm84 merged 1 commit into
feat/fdv2from
mk/NOTICKET/fix-hook-test-fdv2

Conversation

@keelerm84
Copy link
Copy Markdown
Member

@keelerm84 keelerm84 commented May 15, 2026

Summary

  • Cherry-pick of #341 onto feat/fdv2.
  • Adds cross-hook ordering assertions for the three hook stages the harness currently knows about:
    • beforeEvaluation -- registration order.
    • afterEvaluation -- reverse of registration order.
    • afterTrack -- registration order.
  • The harness previously had no ordering coverage for any hook stage. Each Hooks group now owns a shared *atomic.Int64 threaded into every HookCallbackService; the endpoint handler stamps the counter onto each received payload before pushing it onto the channel, so tests can reconstruct cross-hook execution order from the otherwise-independent callback URLs.
  • Also preserves hook registration order through Hooks.Configure (the prior implementation iterated a Go map, which is randomized), so the ordering assertions are deterministic.
  • The new Sequence field on HookExecutionPayload is harness-internal (json:"-") and is not part of the SDK contract; no contract-test changes are required in SDK repos.

Note

Medium Risk
Moderate risk because it changes test-harness hook configuration ordering (from map iteration to explicit order) and introduces new ordering assertions that may fail SDKs with previously-untested behavior.

Overview
Adds ordering coverage for hooks by tracking cross-hook execution order and asserting it in common tests.

Hooks now preserves registration order explicitly when configuring hooks (avoiding randomized Go map iteration), and HookCallbackService can stamp a shared atomic Sequence onto each received HookExecutionPayload (new harness-only field) so tests can sort callbacks across distinct hook endpoints.

Introduces new tests asserting that beforeEvaluation and afterTrack execute in registration order, while afterEvaluation executes in reverse registration order.

Reviewed by Cursor Bugbot for commit e59c949. Bugbot is set up for automated code reviews on this repo. Configure here.

@keelerm84 keelerm84 marked this pull request as ready for review May 15, 2026 21:00
@keelerm84 keelerm84 requested a review from a team as a code owner May 15, 2026 21:00
@keelerm84 keelerm84 merged commit dee6b74 into feat/fdv2 May 15, 2026
8 checks passed
@keelerm84 keelerm84 deleted the mk/NOTICKET/fix-hook-test-fdv2 branch May 15, 2026 21:01
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.

2 participants