feat(stack): run the complete native service graph - #6385
Conversation
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1515d4700
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c912c31e38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a0834d944
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fe089ab55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Automated review convergence checkpoint after three batched fix rounds:
The automated loop is now capped per AGENTS.md because new findings are primarily in code introduced by prior bot-driven rounds. Please provide human review, especially on whether Storage restart persistence should be folded into CLI-2141 before merge or tracked separately. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69acdd0e84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
avallete
left a comment
avallete
left a comment
There was a problem hiding this comment.
Review
Cross-reviewed the stacked diff (ccbf5c5...69acdd0) only: Cursor + Claude Fable + Codex. Graph wiring, companions, journals, version planning, and eager/lazy/explicit prepare closures look coherent. Remaining problems sit at the host-native seam this PR introduced.
Changes requested — three small, diff-local fixes. Storage wipe is a follow-up, not a hold.
Ask before merge
- Native Analytics / BEAM cookies —
analytics.tsstill usesRELEASE_COOKIE: "cookie"on the host EPMD. Thread a random per-stack cookie to Analytics/Realtime/Pooler and bind epmd/dist to loopback. See inline. - Native PgMeta bind — set
PG_META_HOST=127.0.0.1. It is the only native HTTP service left on0.0.0.0. See inline. - Realtime
Hostprobe on Node — native health check setsHost: <tenantId>viafetch. Please confirm Node does not strip it; Bun e2e would miss that. See inline.
Parked (do not hold CLI-2141)
- Storage/Mailpit
orphanCleanupon requested stop/restart is real (supervisor-runtime.tsruns cleanup onShutdownRequested). CLI managed start always passes a persistedstackRoot, sosupabase start/stop/restartdoes not wipe uploads. The wart is programmaticcreateStack()temp roots and mid-sessionrestartService("storage")there — same parent contract already on Postgres. Fix in process-compose:RemovePathonly on owner loss. - imgproxy
IMGPROXY_LOCAL_FILESYSTEM_ROOT: "/"stays with CLI-2227; native blast radius is wider than Docker. start --moderestamping the previous mode's versions is anext/consumer follow-up, not the stack graph.- Realtime
ERL_CRASH_DUMPat cwd, missingruntime.envfootprint profile, and the NativeLogWriter NUL sentinel leaking throughLogBufferare real ownership/DX misses. Follow-up.
Cleanup
No review/tool attributions. Leftovers worth trimming if you are already in the files:
- Pooler native env asserted twice (
services.unit.test.tsandpooler.unit.test.ts). orphanCleanupcopied instorage.ts/mailpit.ts.- Dead aliases after
applyNativeDefaultsremoval (supervisor.tsruntimeConfigInput,SupervisorUpgradeRestart.tseffectiveConfigInput). startNativeLogWriterexported fromeffect.tswith onlyLocalStackas a consumer.- e2e
bindAndCloseafter dispose is released-port reuse; the PID check is enough.
Cross-review logs: Claude Fable + Codex on the stacked head. Nothing else posted besides this review.
69acdd0 to
246d59d
Compare
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@9f3f0fbf21ae9f16b92f51bb8d40008cdac83dbePreview package for commit |
| command: `${opts.binPath}/bin/.edge-runtime-wrapped`, | ||
| args: [...edgeRuntimeArgs(opts, ".")], | ||
| cwd: opts.bootstrapDir, | ||
| env: edgeRuntimeEnv(opts), |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
Native Edge Runtime is launched with only --port and no loopback bind. Deno.serve defaults to 0.0.0.0 (https://docs.deno.com/api/deno/http-server/), exposing the function listener on every host interface instead of only through the loopback API proxy. A network client can directly invoke configured functions, including verifyJWT: false functions.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The root cause is that Deno.serve in edge-runtime-main.ts (line 229) does not specify a hostname, causing it to default to 0.0.0.0 and bind on all network interfaces when running natively on the host OS. To fix this:
- In
edge-runtime-main.ts, modify theDeno.servecall to read a host binding from an environment variable with a loopback default:
Deno.serve({
hostname: Deno.env.get("EDGE_RUNTIME_HOST") ?? "127.0.0.1",
handler: async (req: Request) => { ... },
onListen: async () => { ... },
});- In
edge-runtime.ts, updateedgeRuntimeEnv(or the Docker-specific env override) to explicitly setEDGE_RUNTIME_HOST=0.0.0.0for the Docker variant so container bridge networking continues to work (Docker requires binding on all interfaces inside the container for port publishing to function). The native variant will naturally fall through to the127.0.0.1default, restricting the listener to the loopback interface only and preventing remote clients from directly invoking functions—including those withverifyJWT: false—by bypassing the API proxy.
| readonly releaseCookie?: string; | ||
| }, | ||
| ): Record<string, string> => ({ | ||
| PORT: String(opts.port), |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
Native Realtime is started with PORT but no loopback HTTP bind setting. Its release endpoint therefore listens on host interfaces, allowing a network client to bypass the loopback API proxy and reach /socket and HTTP endpoints on the Realtime port using the publishable key, exposing the local stack beyond its intended boundary.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Add PHX_HTTP_IP: "127.0.0.1" inside the native-mode conditional spread block (lines 76–84) to restrict the Realtime Phoenix HTTP server to the loopback interface only. The existing conditional already distinguishes native from Docker mode via the presence of nodeName/releaseCookie. The fix should be added there alongside the other native loopback bindings:
...(opts.nodeName === undefined || opts.releaseCookie === undefined
? {}
: {
...nativeBeamLoopbackEnv,
NODE_NAME: opts.nodeName,
NODE_IP: "127.0.0.1",
RELEASE_NODE: `${opts.nodeName}@127.0.0.1`,
RELEASE_COOKIE: opts.releaseCookie,
PHX_HTTP_IP: "127.0.0.1",
}),This follows the exact same pattern used by the Analytics/Logflare service in analytics.ts, which sets PHX_HTTP_IP: opts.nodeHost with nodeHost: "127.0.0.1" for native mode (and "0.0.0.0" for Docker, where container-network isolation applies instead). Without this setting, Phoenix defaults to binding on all host interfaces (0.0.0.0), allowing external network clients to reach /socket and HTTP endpoints on the Realtime port directly, bypassing the intended loopback API proxy boundary. Note: confirm that PHX_HTTP_IP is the correct runtime-config variable in Supabase Realtime's config/runtime.exs before merging.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 246d59d7bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // URLs addressable on the host while the data directory remains owned by | ||
| // Storage and is not treated as imgproxy's independent state. | ||
| IMGPROXY_LOCAL_FILESYSTEM_ROOT: "/", | ||
| IMGPROXY_ALLOWED_SOURCES: `local:///${normalizeLocalSourcePrefix(dataDir)}`, |
There was a problem hiding this comment.
When image transformation is enabled, normalizeLocalSourcePrefix returns an absolute path beginning with /, so this produces local:////tmp/... (and local:////var/lib/storage/ in Docker). Storage emits local:///tmp/...-style source URLs, as the adjacent comment notes, so they do not match this URL-prefix allowlist and image transformation requests are rejected. Prefix the absolute path with local://, not local:///.
Useful? React with 👍 / 👎.
| return closeEvent === undefined ? Effect.fail(error) : Effect.succeed(closeEvent); | ||
| }), | ||
| ); | ||
| const bridge = Effect.race(Fiber.join(clientFiber), backendToClient).pipe( |
There was a problem hiding this comment.
When either WebSocket side terminates with a non-close SocketError—for example, the Realtime backend emits a read error after connecting—the corresponding effect fails, but Effect.race is success-biased and continues waiting for the other side instead of returning that failure. The client can therefore remain connected indefinitely and the later 1011 error-close branch is never reached. Use first-completion race semantics or convert each terminal failure into a successful bridge outcome before racing.
Useful? React with 👍 / 👎.
| // Log delivery must never block child stdout/stderr draining. The bounded history refs | ||
| // below remain authoritative; live subscribers receive the newest entries when slow. | ||
| const globalPubSub = yield* PubSub.sliding<LogEntry>(4096); | ||
| const internalPubSub = yield* PubSub.unbounded<LogBufferInternalEvent>(); |
There was a problem hiding this comment.
When native services produce logs faster than the filesystem journal can write them—for example during slow-disk operation or repeated write retries—every entry is retained by this unbounded PubSub. Unlike the existing sliding live-log queues, the journal consumer performs filesystem I/O and can remain behind indefinitely, so a noisy stack can grow the daemon's memory without limit and eventually be killed. Use a bounded nonblocking queue while preserving ordered delivery of the shutdown control marker.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f3f0fbf21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| pgmetaPort: { field: "pgmetaPort", service: "pgmeta", persistence: "runtime" }, | ||
| // postgres-meta binds its administrative listener at PG_META_PORT + 1. | ||
| pgmetaPort: { field: "pgmetaPort", service: "pgmeta", span: 2, persistence: "runtime" }, |
There was a problem hiding this comment.
When a Docker stack enables PgMeta, this span requires both the configured port and port + 1 to be free, even though makePgmetaServiceDocker publishes only the configured port. A process occupying the adjacent host port therefore rejects an otherwise valid Docker start or restart, and the lease unnecessarily prevents sibling stacks from selecting that unused port. Reserve the two-port span only for native PgMeta, where the administrative listener actually binds on the host.
Useful? React with 👍 / 👎.
| const result = handler(chunk); | ||
| if (Effect.isEffect(result)) { | ||
| run(result); |
There was a problem hiding this comment.
When a client sends frames while lazy Realtime activation or the backend handshake is still pending, every message event forks another effect that retains its payload while waiting on backendWriter; there is no queue bound or backpressure. A fast or faulty loopback client can therefore create an unbounded number of fibers during the potentially minutes-long activation window and exhaust the daemon's memory. Feed frames through a bounded, serialized queue and close the connection on overflow instead of forking once per event.
AGENTS.md reference: AGENTS.md:L142-L146
Useful? React with 👍 / 👎.
2 participants
Summary
Extends the strict native stack runtime from the Postgres/Auth/PostgREST core to the complete service graph using the frozen slim-services releases.
Adds native launch, configuration, lifecycle, and private companion ownership for Edge Runtime, Realtime, Storage/imgproxy, PgMeta/Studio, Analytics/Vector, Pooler, and Mailpit. Native logs remain supervisor-owned and isolated per stack, while the public package surface preserves eager, lazy, and explicit preparation closure semantics.
The representative consumer journey covers the public service graph and exact resource ownership. This change does not add per-service Docker fallback or absorb unrelated proxy hardening.