ProviderModelNotFoundError

github run

@kentonv Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

/bonk retry review

Stale comment in worker-interface.capnp: Lines 596-597 still say:

Note that externals must be pushed BEFORE the JsValue that uses them is sent, so that they can be unwrapped immediately when deserializing the value.

This is no longer accurate after this PR — the implementation now handles either ordering. Consider updating this comment.

Review posted on PR #6607. Here's a summary of my analysis:

Overall: This is a well-structured fix. The core change replaces tryGetLocalServerSync() (which required the push RPC to arrive before unwrap) with async getLocalServer() (which waits for the push if needed). The approach is sound.

Key observations:

1. Stream unwrap (unwrapStreamImpl): Uses kj::newPromisedStream() to return a synchronous kj::Own<kj::AsyncInputStream> backed by a promise. The async resolution via getLocalServer() is correctly hidden behind the promised stream. No reference lifetime issues — unwrapped is used immediately after co_await with no intervening suspension points.

2. AbortSignal unwrap (unwrapAbortSignalImpl): The deferred trigger pattern is clever — pushAbortSignal() returns a promise-for-capability as the trigger, and unwrapAbortSignalImpl() fulfills it later with the actual AbortTriggerRpcServer once it can allocate the PendingAbortReason synchronously. The inner block scoping correctly ensures unwrapped and cap don't outlive their valid use, and the co_await paf.promise at the end keeps the coroutine alive until the abort signal fires. No dangling reference issues.

3. HACK removal in worker-rpc.c++: The extra event-loop turn that was inserted to ensure push-before-unwrap ordering is no longer needed since the implementation now handles either ordering. Clean removal.

Two minor findings posted:

• Stale comment in worker-interface.capnp (lines 596-597) still says externals must be pushed before the JsValue — no longer accurate.
• Typo "the the" in external-pusher.c++ line 211.

github run