[chore] Extend ngrok to split general ingress and mounts - #6010
Conversation
…s by upstream The development tunnel published the object store, and nothing published the API. Two things now need the opposite: a platform posting a webhook, and an authorization server fetching a document we serve. It forwards to traefik instead, so every inbound route arrives on its normal path -- /api/... is already routed there, in the self-host compose files, and in production, so no route needs a tunnel of its own. NGROK_DOMAIN pins a reserved domain, which is what keeps an address handed to a provider valid across restarts. A browser redirect never needed this. The user is already looking at the deployment, so the address that got them there is one their browser reaches. discoverTunnelEndpoint returned the first https tunnel without checking what it forwarded to, which was correct only while exactly one tunnel existed and it happened to be the store's. It now matches on the upstream host and port and returns nothing when no tunnel serves the store, because handing back another tunnel's URL would mount an HTTP API as an object store and fail far from its cause. The caller already refuses the mount and tells both the operator and the model why. One configuration loses a convenience, stated rather than discovered: Daytona sandboxes with the bundled in-network store no longer get a durable working directory. Point the store at a public endpoint -- production runs no bundled store at all -- or run a second tunnel for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Matches the project-workspace convention the other design workspaces use. No work packages: the tunnel target, the selector fix and the capability that moves only make sense together, and landing the repoint without the selector would leave it returning the wrong URL. The tasks list carries the deploy-and-verify checks, each one chosen because it can fail quietly -- including that a refused sandbox mount must be audible, and that a Daytona run against a public store must keep working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…re's Corrects the previous commit, which repointed the store's tunnel at the ingress instead of adding one. That silently cost Daytona sandboxes with the bundled store their durable working folder, and it was not a trade anyone asked for. The store tunnel is restored byte-for-byte from main: same target, same comments, same depends_on. The compose diff is purely additive -- zero removed lines in either edition. The new ngrok-api service forwards to traefik, so anything outside that must reach us arrives on its normal path: /api/ is already routed in dev, in the self-host files, and in production, so one endpoint serves channels, the model and MCP gateways, and whatever follows. NGROK_API_DOMAIN pins a reserved domain, which is what keeps an address handed to a provider valid across restarts. Two tunnels means two agent sessions; if a plan allows only one, a single agent with two named endpoints works too, and the selector fix is what makes that safe. That selector took the first https tunnel regardless of what it forwarded to -- correct only while one tunnel existed, which is exactly what this change ends. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ngrok becomes ngrok-fs, beside the new ngrok-api. Two tunnels with one generic name and one specific one reads as if the first were the default and the second an afterthought; both are peers. The store tunnel keeps its target, its token gate and its comments. Only the name moves -- and with it the one thing that addresses it: nothing sets AGENTA_MOUNTS_TUNNEL_API, so the runner reaches the agent through a compiled-in default, now http://ngrok-fs:4040. The variable stays an override, and the service name is load-bearing because of it. An operator with a stack already up gets an orphaned ngrok container; the deploy list says to pass --remove-orphans once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ngrok-fs becomes ngrok-mounts and ngrok-api becomes ngrok-ingress. The second tunnel forwards to traefik, so it publishes the whole deployment and not just the API; its old name claimed otherwise. NGROK_API_DOMAIN follows as NGROK_INGRESS_DOMAIN. Drop AGENTA_MOUNTS_TUNNEL_API. It overrode the address of the tunnel daemon the runner queries for the store's public URL, was never set in any compose file, env example or deployment, and the compose service name it defaulted to already resolves. Tests inject through the deps seam, which stays. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l domain NGROK_INGRESS_DOMAIN becomes NGROK_DOMAIN_INGRESS. Vendor-prefixed vars put the attribute before the instance -- REDIS_URI_DURABLE and REDIS_URI_VOLATILE beside a shared REDIS_URI, POSTGRES_URI_CORE and its siblings. Our own AGENTA_* namespace narrows the other way, which is what the first name followed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tunnel vars sat unbannered at the tail of the Daytona section. Now that there are two of them driving two services, they get a header like every other vendor block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the local dev stack to support two ngrok tunnels: one for the object-store mounts used by remote sandboxes and a second for general ingress (Traefik) so external providers can reach /api/... webhooks during development. It also hardens the runner’s tunnel discovery so it selects the correct tunnel when multiple are present.
Changes:
- Add
ngrok-ingress(tunnelstraefik:80) and rename the existing store tunnel service tongrok-mounts. - Update
discoverTunnelEndpointto match tunnels by upstream (store endpoint) and wire the store endpoint through the call sites; add unit tests. - Remove
AGENTA_MOUNTS_TUNNEL_APIusage and document the new optionalNGROK_DOMAIN_INGRESS.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/runner/tests/unit/sandbox-agent-mount.test.ts | Adds tests for multi-tunnel selection behavior and updates the default ngrok API hostname used in tests. |
| services/runner/src/engines/sandbox_agent/mount.ts | Renames default ngrok API to ngrok-mounts and matches tunnels by upstream endpoint for correctness with multiple tunnels. |
| services/runner/src/engines/sandbox_agent/environment.ts | Passes storeEndpoint into tunnel discovery at both call sites. |
| hosting/docker-compose/oss/env.oss.dev.example | Documents the two-tunnel setup and introduces optional NGROK_DOMAIN_INGRESS. |
| hosting/docker-compose/oss/docker-compose.dev.yml | Renames ngrok → ngrok-mounts and adds new ngrok-ingress service forwarding to Traefik. |
| hosting/docker-compose/ee/env.ee.dev.example | Same env documentation updates as OSS. |
| hosting/docker-compose/ee/docker-compose.dev.yml | Same compose service rename/addition as OSS. |
| docs/designs/platform/store-generalization/tasks.md | Notes that AGENTA_MOUNTS_TUNNEL_API guidance has been superseded by later removal. |
| docs/designs/platform/store-generalization/specs.md | Adds superseding note documenting removal rationale for AGENTA_MOUNTS_TUNNEL_API. |
| docs/designs/platform/runner-rename/specs.md | Notes AGENTA_MOUNTS_TUNNEL_API mention is superseded by later removal. |
| docs/design/dev-ingress/tasks.md | Adds task checklist for the dev ingress tunnel change and verification steps. |
| docs/design/dev-ingress/specs.md | Adds design spec for two tunnels and the upstream-matching selector behavior. |
| docs/design/dev-ingress/research.md | Adds research notes motivating why ingress tunneling is needed (provider callbacks) and why store tunneling can’t share a host by path. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
One `ngrok http` can be listed twice by the agent API, once as http and once as https over the same upstream. The upstream match took whichever came first, so geesefs could reach the store unencrypted over the internet. The unmatched path already preferred https; this makes both paths agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A caller that supplied an endpoint we could not parse fell through to "first https tunnel wins", which could return the ingress tunnel. That is the failure the upstream matching exists to prevent. Return null and let the caller skip the mount with a named cause. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Context
Development has one ngrok tunnel. It points at the object store, so that Daytona sandboxes can mount their durable working folder over a public URL. Nothing publishes the deployment itself.
That blocks every integration where a provider has to call us. Slack posts events and interactions to a Request URL. An authorization server fetches a client identity document from us. Neither can reach a local stack. The two workarounds we already have do not generalize: Stripe uses its own CLI to forward events, and Composio subscribes over its own WebSocket. Both depend on the provider offering a subscribe call, and an arbitrary platform offers nothing equivalent.
Changes
A second tunnel,
ngrok-ingress, forwards totraefik:80. It publishes the whole deployment rather than one service, so every inbound route arrives on its normal path./api/is already routed in development, in the self-host compose files, and in production, which means no integration needs a tunnel or a route of its own.ngrokis renamed tongrok-mounts, so each service is named for what it publishes. Its target, token gate and startup behaviour are unchanged. The runner addresses that service by name, so its compiled in default moves with it.The tunnel selector now matches on upstream instead of list order. This is the part worth reviewing.
discoverTunnelEndpointreturned the first https tunnel it found, with no check on what that tunnel forwarded to. That was correct only while exactly one tunnel existed.Before:
After:
Without this, adding a second tunnel could hand a sandbox the platform's HTTP API as an object store, and that failure would surface far from its cause. Returning null is already handled: the caller skips the mount, warns the operator, and tells the model the durable folder is unreachable this turn. Callers that pass no store endpoint keep the old behaviour.
AGENTA_MOUNTS_TUNNEL_APIis removed. It overrode the address of the ngrok daemon the runner queries. It was never set in any compose file, env example or deployment, and the compose service name it defaulted to already resolves. Tests inject through the existingdepsseam, which stays.NGROK_DOMAIN_INGRESSis new and optional. Without it the public address rotates on every restart, which matters because that address gets registered by hand in a provider's dashboard. There is no matching variable for the store tunnel on purpose: the runner rediscovers that address at the start of every run, so nothing outside remembers it.Design notes for the change live under
docs/design/dev-ingress/.Tests and notes
pnpm testinservices/runner: 2117 passed. The 19 failures incommit-authorization,sandbox-agent-acp-interactionsandworkspace-importare pre existing, confirmed by running the suite on the base commit with this work stashed.pnpm run typecheckis clean.docker compose configvalidates for both editions, and with thewith-tunnelprofile each lists exactlyngrok-mountsandngrok-ingress.ngrok-mountsand reads its API, and/api/healthanswers 200 through the ingress tunnel.What to QA
Written for a clean start: fresh checkout, no existing stack.
Setup. Copy the edition's
env.*.dev.exampleto your env file and setNGROK_AUTHTOKEN. SetNGROK_DOMAIN_INGRESStoo if you have a reserved domain. Both now live under their ownngroksection. Bring the stack up as usual. The tunnel profile is on by default, so nothing extra is needed to get both tunnels.docker psshowsngrok-mountsandngrok-ingressboth up. Two tunnels need two ngrok agent sessions, so watch for the second failing to start on a restricted plan. That is the most likely way this fails on someone else's account.curl https://<ingress-url>/api/healthanswers{"status":"ok"}as JSON. HTML means it reachedweband the path is wrong. Get the address from the ngrok dashboard or thengrok-ingresscontainer logs.mount SKIPPEDwarning. An earlier draft of this change broke exactly this.NGROK_AUTHTOKENunset, neither service publishes anything and neither loops. Each should exit 0 once and stay exited.Only if you are upgrading a stack that is already running, pass
--remove-orphanson the first bring up. The service rename leaves an orphanedngrokcontainer behind otherwise, which is confusing rather than harmful. A clean start never sees this.