[strandly-the-agent]

Requested by: @ekumar96 — cut PRs for the ContextOffloader / SummarizingConversationManager fixes blocking the AgentCore Harness launch (Slack, 2026-10-27).

Human Overview

Context offloader now runs after default hooks, so they don't see offloaded tool result. This is an outdated behavior that doesn't happen on context manager.

Description

ContextOffloader._handle_tool_result replaced event.result with the preview before any hook passed via hooks= ran: plugins register after hooks=, and AfterToolCallEvent reverses registration order within a HookOrder group, so the offloader always ran first and audit/DLP hooks only saw the preview.

The handler is no longer @hook-discovered; init_agent registers it with agent.add_hook(..., AfterToolCallEvent, order=70). Group order is ascending and reversal only applies within a group, so the rewrite now runs after every DEFAULT hook. Per mkmeral: not SDK_LAST (SDK components shouldn't occupy the ends of the scale) and no new HookOrder constant (hook ordering is slated for deprecation in favour of the context manager). 70 sits between MODEL_ROUTING (50) and INTERVENTION_INPUT (90) with room on both sides. No public API change.

Related Issues

Fixes #5596

Overlaps with #5599 (MuonChaser, uses SDK_LAST, opened independently) — maintainers can keep whichever they prefer.

Documentation PR

None needed (docs don't describe hook ordering).

Type of Change

Bug fix

Testing

• New: test_init_agent_registers_tool_result_hook_last (asserts the order is strictly between DEFAULT and SDK_LAST) and test_user_after_tool_call_hooks_see_full_result_before_rewrite (real Agent + hooks= callback + offloader; the hook sees the 2,000-char result, the stored message holds the preview). Both fail on main.

• pytest tests/strands/vended_plugins/context_offloader tests/strands/hooks tests/strands/agent/test_agent_delegation.py: 264 passed (rebased on current main). ruff check/format clean; mypy clean on strands.hooks + strands.vended_plugins.context_offloader.

• I ran hatch run prepare (sandbox has no hatch; ran the equivalent ruff/mypy/pytest directly)

Additional Details

Independent review (fresh-context reviewer, round 1) and ledger

Verdict: approve. Checked: no other AfterToolCallEvent registration above DEFAULT; interventions (INTERVENTION_OUTPUT, -90) still run before the offloader; delegation's _on_after_tool_call reads only status; no double registration (one init_agent per agent, plugin names unique). The later SDK_LAST → 70 change doesn't alter any of these relationships.

• 🔵 docstring narrated the alternative (rather than via @hook) — fixed.
• 🔵 (optional, pre-existing) the offloader could skip results whose hook set event.retry — not taken; unrelated to this bug.
• Follow-up: TS vended-plugins/context-offloader/plugin.ts has the same ordering bug — tracked in the [PORT] issue.

Checklist

• I have read the CONTRIBUTING document
• I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works (human attestation pending)
• My change is focused and reasonably small; I have split unrelated work into separate PRs
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[mkmeral]

@strandly-the-agent update the pr. I don't actually want any component in the SDK to call SDK last or first. bc if we need sth to come afterwards, we'll need to move. I'd rather leave some gaps, so we can update later without changing bunch of constants. Check the ordering we have so far, and come up with a better number

[strandly-the-agent]

Updated (842a73a): added HookOrder.CONTEXT_OFFLOAD = 70 and registered the offloader there instead of SDK_LAST, following the MODEL_ROUTING = 50 precedent.

Why 70

Current scale: SDK_FIRST -100, INTERVENTION_OUTPUT -90, DEFAULT 0, MODEL_ROUTING 50, INTERVENTION_INPUT 90, SDK_LAST 100.

On AfterToolCallEvent the only other SDK orders are INTERVENTION_OUTPUT and DEFAULT, so anything > 0 fixes the bug. 70 sits between the two existing neighbours and leaves 51–69 and 71–89 open on both sides of the rewrite. Happy to shift it if you'd rather it closer to 50.

FYI the other existing SDK_LAST/SDK_FIRST users in-tree (router.py, triggers.py, _background_tasks.py, snapshot_session_manager.py, _agent_delegation.py) are untouched here — separate cleanup if you want it. The TS port should add the same constant to hooks/types.ts.

Rebased on main; 264 tests pass across offloader/hooks/delegation, ruff + mypy clean.

[mkmeral]

@strandly-the-agent I don't want to add the number in hook orders. we will deprecate this soon, and move to context manager, and i dont want people to get confused. let's take this as a bugfix and coninue imo

[strandly-the-agent]

Done (0717df0): HookOrder.CONTEXT_OFFLOAD removed, registry.py untouched. The offloader registers with a plain order=70 and a one-line comment; the registration test asserts DEFAULT < order < SDK_LAST rather than the literal. Diff is back to just plugin.py + its test. 264 tests pass, ruff + mypy clean.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

Assessment: Approve

Clean, well-scoped bug fix. I traced the ordering mechanism and confirmed the fix is correct and robust: AfterToolCallEvent.should_reverse_callbacks is True, and get_callbacks_for iterates order groups ascending while reversing only within a group — so registering _handle_tool_result at order=70 guarantees it runs after the DEFAULT (0) group regardless of intra-group registration order. The audit/DLP hooks passed via hooks= now see the full result before the rewrite.

What I verified

• Ordering on AfterToolCallEvent: INTERVENTION_OUTPUT (-90) → DEFAULT (0) (user hooks=, delegation _on_after_tool_call) → offloader (70). INTERVENTION_INPUT (90) is unused on this event. Delegation reads only status, so it's unaffected.
• No competing registration above DEFAULT on this event; no double registration — @hook is dropped from _handle_tool_result (auto-discovery now yields only _on_before_model_call) and init_agent adds it once per agent.
• Tests: both new tests fail on main and pass here (91 offloader + 76 hooks/delegation, after installing the source tree editable). test_init_agent_registers_tool_result_hook_last correctly asserts the DEFAULT < order < SDK_LAST range rather than the literal; the regression test references [BUG] ContextOffloader rewrites the tool result before other AfterToolCall hooks see it #5596.
• Comments/docstrings align with the repo's evergreen-comment guidance (state the why, no change-narration).
• The order=70 / no-new-constant decision was already settled with @mkmeral above — not relitigating.

One optional, non-blocking note: order=70 fixes the common case because user hooks= register at DEFAULT. A consumer who explicitly registers an audit/DLP hook at order > 70 would still observe the preview — acceptable given hook ordering is slated for the context-manager replacement, just worth being aware of.

Nice, tightly-scoped fix with regression tests that actually pin the ordering contract.