Revert PR #54 "fix(executor): pass ownerId via per-call props"#55
Merged
jonnyparris merged 1 commit intomainfrom Apr 27, 2026
Merged
Revert PR #54 "fix(executor): pass ownerId via per-call props"#55jonnyparris merged 1 commit intomainfrom
jonnyparris merged 1 commit intomainfrom
Conversation
This reverts commit 3f7e774b... at runtime with:
outbound is not a function
The LoopbackServiceStub callable signature (env.OUTBOUND({ props })) is
ONLY available via ctx.exports, not via wrangler service bindings. A
regular service binding to your own Worker still gives you a plain
Fetcher with only .fetch() and .connect() — no per-call props
callable.
The proper fix needs to plumb ctx.exports.AllowlistOutbound from the
top-level fetch handler (which has ExecutionContext) down through
CodingAgent (a DurableObject, no ctx.exports) to runSandboxedCode.
That's a bigger architectural change than what shipped here — back to
the drawing board.
(Third attempt at this same fix. Pattern: types pass, unit tests pass,
runtime fails. Live testing before merge would have caught all three —
adding it to the workflow checklist.)
jonnyparris
added a commit
that referenced
this pull request
Apr 27, 2026
The duck-typed wrapper in wrapOutboundWithOwner returned a plain JS object cast 'as Fetcher', which workerd rejects at WorkerCode validation with: Incorrect type for the 'globalOutbound' field on 'WorkerCode': the provided value is not of type 'Fetcher'. This broke every codemode sandbox fetch (and clone via codemode) for any session with an ownerId — i.e. all logged-in users. Three prior fix attempts (#52 LOADER.get proxy, #54 ctx.exports per-call props, #55 revert) hit different runtime/architectural walls. This change removes the wrapper entirely and passes env.OUTBOUND directly to DynamicWorkerExecutor. workerd accepts it because OUTBOUND is a real service-binding ServiceStub. Trade-off: per-user GitHub/GitLab token injection for raw sandbox fetch() is no longer wired up. AllowlistOutbound continues to enforce the hostname allowlist, so the perimeter is unchanged. Sandbox code that needs auth must include its own headers — but the sandbox no longer has direct access to user secrets, which is the safer default. Git operations are unaffected because resolveRemoteToken() runs in the parent DO and tokens are passed straight to isomorphic-git. System prompts updated to reflect the new contract. Old outbound.test.ts (17 tests for the deleted injectAuth helper) replaced with 4 tests covering the constants and stripping behaviour that survive.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverts #54. Third broken attempt at this fix.
What happened
Live test from a fresh codemode session after deploying `3f7e774`:
```
Root cause
The `LoopbackServiceStub` type with the callable `({ props })` signature is only materialized via `ctx.exports[name]`. A regular service binding declared in `wrangler.jsonc` (even when targeting your own Worker's `AllowlistOutbound` entrypoint) gives you a plain `Fetcher` — `.fetch()` and `.connect()` only. No callable.
I had typed `env.OUTBOUND` as `LoopbackServiceStub` based on the type definitions, but the workerd runtime doesn't actually wire that callable through for env-bindings. TS lied; production told the truth.
Path forward
The proper fix needs to plumb `ctx.exports.AllowlistOutbound` from the top-level worker fetch handler (where `ExecutionContext` is available) down through CodingAgent (a DurableObject, where `ctx` is `DurableObjectState` — no `exports`) to `runSandboxedCode`. That's a more invasive architectural change than what shipped here.
Score so far on this fix
Adding "live test before merging Workers infra fixes" to the workflow checklist permanently. See `memory/learnings.md` on agent-hq.
🤖 generated by ruskin's agent