Skip to content

callback_wrapper middleware around every user-callback dispatch (#47 phase 2) - #49

Merged
ya-luotao merged 2 commits into
mainfrom
feat/callback-wrapper
Jul 31, 2026
Merged

callback_wrapper middleware around every user-callback dispatch (#47 phase 2)#49
ya-luotao merged 2 commits into
mainfrom
feat/callback-wrapper

Conversation

@ya-luotao

Copy link
Copy Markdown
Owner

Summary

Implements phase 2 of #47 (Option D): ClaudeAgentOptions#callback_wrapper — optional middleware composed around every user-callback dispatch through FiberBoundary (message blocks, observers, hooks, permission callbacks, SDK MCP tool/resource/prompt handlers), in both :thread and :inline modes.

ClaudeAgentSDK.configure do |config|
  config.default_options = {
    callback_wrapper: ->(inv) { Rails.application.executor.wrap { inv.call } }
  }
end

The wrapper runs on the same execution context as the callback — inside the worker thread in :thread mode (so executor.wrap checks ActiveRecord connections back in when the callback ends, retiring the stranded-connection workaround without adopting inline), in place on the reactor fiber in :inline mode. Also a generic hook for APM/logging context propagation. Default nil — zero behavior change.

Design points

  • Composed before the thread hop in FiberBoundary.invoke — that ordering is the entire feature.
  • Break is invisible to wrappers: the LocalJumpError → Break translation lives inside the invocation handed to the wrapper, so a conforming rescue/report/re-raise wrapper observes a clean return on a user's break, never an exception it could falsely report — and a swallowing wrapper cannot lose the break.
  • Travels with the scheduling mode through the existing SchedulingScope fiber-storage carrier to shared SDK MCP servers; the pair is resolved atomically (effective_callback_dispatch, one liveness decision) so a scope closing mid-resolution can never yield a torn session-mode/server-wrapper mix. Server instances are never mutated; descendants outliving the dispatch fall back to server defaults — all phase-1 invariants preserved.
  • Inline hook timeouts (InlineCancellation < Exception) pass through StandardError-rescuing wrappers un-swallowed.
  • The timeout-bounded store-adapter path deliberately ignores wrappers (hard-timeout carve-out, unchanged).

Process

Implemented by a Claude Code agent against a written spec, reviewed by me, then adversarially reviewed by codex (gpt-5.6-sol ultra) — which found two real defects (torn pair resolution under concurrent scope closure, reviewer-reproduced; break exposed to wrappers as LocalJumpError), both fixed in 14a6fa9 and re-verified by the reviewer's own probes. Final verdict: SHIP.

Testing

  • New spec/unit/callback_wrapper_spec.rb (37 examples): exactly-once wrapping per callback type in both modes, wrapper-on-worker-thread / wrapper-on-reactor-fiber assertions, return value passthrough + exception propagation (incl. wrapper-raised), break through wrappers (ensure-based, reporting, swallowing), InlineCancellation pass-through, shared-server cross-session wrapper isolation, descendant-fiber fallback, torn-pair regression (deterministic mid-resolution flip), direct-call defaults, validation, global config.
  • Full suite: 1182 examples, 0 failures — including order-sensitive seeds 62610/48816; rubocop clean.

Closes the phase-2 item of #47. Refs #47.

🤖 Generated with Claude Code

https://claude.ai/code/session_019Manwfh1GRrJtQfgTm1dtt

ya-luotao and others added 2 commits July 31, 2026 14:20
 phase 2)

ClaudeAgentOptions#callback_wrapper (default nil) installs a callable
around EVERY user-callback dispatch crossing FiberBoundary — message
blocks, observers, hooks, permission callbacks, SDK MCP tool/resource/
prompt handlers. Shape:

  callback_wrapper: ->(inv) { Rails.application.executor.wrap { inv.call } }

Design choices:

- Composed BEFORE the thread hop: FiberBoundary.invoke builds the body
  as wrapper.call(block) and hands THAT to Thread.new, so the wrapper
  runs on the callback's own execution context — the worker thread in
  the default :thread mode (executor.wrap must run on the thread that
  touches ActiveRecord, so connections check back in when the callback
  ends; retires the stranded-AR-connection workaround without adopting
  :inline), in place on the reactor fiber in :inline mode.
- invoke_iteration wraps only the args-applied user call; the
  LocalJumpError -> Break translation stays OUTSIDE the wrapper, so a
  user break in :thread mode passes through the wrapper (running its
  ensure) before being bridged. In inline/no-scheduler mode break
  unwinds natively through the wrapper's stack — ensure-based wrappers
  (executor.wrap) are safe; wrappers must not swallow exceptions.
- Exceptions from the callback propagate through the wrapper unchanged;
  wrapper-raised exceptions are treated exactly like callback errors.
  Inline hook-timeout cancellation (InlineCancellation, not a
  StandardError) passes through even a rescue-StandardError wrapper.
- The timeout-bounded (store-adapter) invoke path ignores wrappers:
  that hard-bound carve-out never carries user callbacks.
- SDK-MCP shared-server path: SchedulingScope now carries (mode,
  wrapper) as one closable unit through fiber storage, so concurrent
  sessions with different wrappers sharing one SdkMcpServer never
  mutate it, and descendants outliving a dispatch fall back to the
  server's own defaults (new callback_wrapper accessor +
  effective_callback_wrapper, mirroring the scheduling pair exactly).
- Global default comes free via configure/default_options, same as
  callback_scheduling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019AeLMsroeCAuSLvTbxxnq2
…e to wrappers

Two defects from adversarial review of the callback_wrapper branch (both
reviewer-reproduced):

1. High: effective_callback_scheduling and effective_callback_wrapper
   each decided scope liveness independently — a scope closing between
   the two reads (dispatch ensure racing a descendant reader) yielded a
   torn pair: session :inline mode with the server's direct-call wrapper.
   effective_callback_dispatch now decides liveness ONCE and returns
   [scheduling, wrapper] atomically; all four server call sites use it,
   and the individual accessors delegate to it. Regression test forces
   the mid-resolution flip deterministically.

2. Medium: a user's `break` in a message block reached the wrapper as
   LocalJumpError — a conforming rescue/report/re-raise wrapper falsely
   reported normal loop control as an error, and a swallowing wrapper
   lost the break entirely. The LocalJumpError -> Break translation now
   lives INSIDE the invocation handed to the wrapper, so wrappers observe
   a clean return. Regression tests: reporting wrapper sees no exception;
   break survives a StandardError-swallowing wrapper.

1182 examples, 0 failures (incl. seeds 62610/48816); rubocop clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Manwfh1GRrJtQfgTm1dtt
@ya-luotao
ya-luotao merged commit 14cccec into main Jul 31, 2026
3 checks passed
ya-luotao added a commit that referenced this pull request Jul 31, 2026
callback_wrapper middleware around every user-callback dispatch — Rails
executor.wrap support on the default :thread mode (#47 phase 2, PR #49).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Manwfh1GRrJtQfgTm1dtt
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