chore: enable Effect linting across monorepo - #6304
Conversation
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@d12a47a5f6244b808c628f8fb91e5b94a3533c25Preview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edcae5572d
ℹ️ 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: e49fe7db95
ℹ️ 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: fa1b421c7f
ℹ️ 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 Reviewcli/apps/cli/src/shared/auth/jwks.ts Lines 266 to 268 in 2aefb88 When an OIDC discovery or JWKS endpoint returns headers but stalls while sending its JSON body, the 10-second timeout has already completed because it wraps only AGENTS.md reference: apps/cli/AGENTS.md:L52-L53 cli/apps/cli/src/shared/functions/functions-docker.ts Lines 327 to 329 in 2aefb88 When AGENTS.md reference: apps/cli/AGENTS.md:L52-L53 cli/apps/cli/src/legacy/shared/legacy-db-dns.ts Lines 92 to 93 in 2aefb88 When the DNS-over-HTTPS server or an intervening proxy returns status 200 and then stalls the response body, this second AGENTS.md reference: AGENTS.md:L68-L72 cli/apps/cli/src/shared/runtime/command-runtime.layer.ts Lines 8 to 11 in 2aefb88 At fleet-scale CLI volume, replacing the UUID with one signed 32-bit AGENTS.md reference: apps/cli/AGENTS.md:L52-L53 When a third-party issuer publishes a valid JWK that relies on members outside this local signing-key schema, such as an RSA key represented through AGENTS.md reference: apps/cli/AGENTS.md:L52-L53 When a short-lived command ends immediately after its final span, AGENTS.md reference: AGENTS.md:L115-L117 cli/packages/stack/src/platform-bun.ts Line 103 in 2aefb88 When a stale daemon or another listener on a deterministic control port returns 200 headers but never completes its owner-status body, this 500 ms timeout ends as soon as ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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 round 3 triage (final bounded pass): Fixed in 435a2d3:
Confirmed but deferred from this PR after reaching the automated-review loop cap:
These deferrals are recorded here for human review rather than silently dropped. The selected fixes have focused regression coverage and the affected CLI/stack quality plus unit/integration gates pass locally. No further automated fix rounds are planned; this is ready for human review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 435a2d36f3
ℹ️ 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".
|
Final automated-review pass, including #6304 (comment), is addressed by 8bb6933.\n\nI validated each report against the actual Effect execution path and the previous Go behavior before changing code. All seven items in that report were reproducible or represented concrete compatibility regressions:\n\n- JWKS timeout now covers request execution, status validation, and JSON body decoding.\n- Resolved project environment values, including BITBUCKET_CLONE_DIR, now reach both function volume creation and bind-mount decisions in deploy, download, serve, and start.\n- DNS fetch and body decoding share one cancellation signal.\n- Command correlation IDs use the injected Crypto service for UUIDv4 values.\n- Remote JWK JSON is preserved losslessly, matching the Go RawMessage behavior; local signing keys remain strictly validated.\n- NDJSON export fibers are scoped, owned, and drained before teardown.\n- Bun control-request timeout now covers JSON body decoding too.\n\nThe four newer inline findings are also addressed: empty SUPABASE_ENV defaults to development, known environment-override validation failures stay in the typed error channel, the duplicate JWKS timeout report is covered by the same fix, and Node/Bun config facades preserve explicit empty ambient values.\n\nI considered pushback explicitly. None was warranted after reproduction: the two initially ambiguous cases—remote JWK extension fields and empty ambient environment values—were confirmed by the Go source and a direct config-loader reproduction. The fixes remain narrow; no compatibility shim or speculative abstraction was added. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bb69332f2
ℹ️ 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: 757ca02d82
ℹ️ 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".
|
Final bounded automated-review pass is complete at cbd93af. Accepted and fixed:
Rejected after reproduction/architecture triage:
The earlier top-level review at issuecomment-5386062423 was addressed separately in 8bb6933 and documented in issuecomment-5387281854. All review threads are resolved, the local no-cache quality/unit/integration matrix passes, and the fresh GitHub code-quality, unit/integration, e2e, and preview workflows are green. We have reached the automated-review loop cap. Requesting human review; further automated findings should be triaged as signals rather than applied automatically. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbd93af8a3
ℹ️ 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".
cbd93af to
214426e
Compare
💡 Codex ReviewReplace the direct AGENTS.md reference: AGENTS.md:L169-L175 Replace the direct AGENTS.md reference: AGENTS.md:L169-L175 cli/apps/cli/src/legacy/auth/legacy-credentials.layer.unit.test.ts Lines 588 to 589 in 214426e Use AGENTS.md reference: AGENTS.md:L169-L176 cli/apps/cli/src/shared/functions/serve.ts Lines 344 to 345 in 214426e Replace these direct AGENTS.md reference: AGENTS.md:L169-L178 cli/apps/cli/src/next/commands/start/ui/dashboard-state.ts Lines 44 to 48 in 214426e Handle only the expected stream failure here and return interruption or defects unchanged with AGENTS.md reference: AGENTS.md:L157-L165 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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: 2edae15c6c
ℹ️ 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 P1 cleanup pass is complete in a7c282d. I accepted the five findings in issuecomment-5388858414 after checking their actual control-flow impact: both missing-file paths and combined unlink errors now use supported tagged predicates; the credentials tests use Exit helpers throughout; filesystem watch events are matched exhaustively; and dashboard state catches only typed stream failures so interruption and defects retain their semantics. The two newer inline regressions are also fixed: compiled builds restore their immutable build ID define, and injected project environment capture preserves the established unprefixed Kong and Vector overrides. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7c282d0b3
ℹ️ 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: d12a47a5f6
ℹ️ 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".
| @@ -760,7 +761,6 @@ export const loadProjectConfigFile = Effect.fnUntraced(function* ( | |||
| options?.projectEnv ?? | |||
| (yield* loadProjectEnvironment({ | |||
| cwd: projectRoot, | |||
There was a problem hiding this comment.
Pass ambient env into direct config loads
When an Effect-native caller invokes loadProjectConfig without options.projectEnv, shell-only variables are no longer included in interpolation. Direct callers such as inferFunctionsManifest and the next-shell start/status handlers still use that form, so a config containing a numeric value such as port = "env(SUPABASE_DB_PORT)" now fails decoding when the variable exists only in the shell, while string references remain unresolved. The Node/Bun loadProjectConfig facades compensate for this, but loadFunctionsManifest and these internal callers bypass that facade; materialize the injected environment at their composition boundary and pass the resulting projectEnv.
AGENTS.md reference: AGENTS.md:L101-L108
Useful? React with 👍 / 👎.
| if (params.scenario.name !== null) { | ||
| params.scenario.log.push({ request: normalized.request, response: normalized.response }); | ||
| yield* writeScenarioInteractions( |
There was a problem hiding this comment.
Serialize all scenario fixture writes
When record mode handles concurrent requests for different normalized fixture keys, each request runs under a different key-specific semaphore but both mutate the same scenario.log and asynchronously overwrite the same interactions.json. Even without teardown, one writer can encode an older snapshot and finish after a newer writer, dropping the later interaction or interleaving file writes; the previous synchronous write could not yield between the append and overwrite. Use a scenario-wide semaphore or a single writer for the shared scenario log and file.
AGENTS.md reference: AGENTS.md:L118-L122
Useful? React with 👍 / 👎.
| home: | ||
| home !== undefined && home.length > 0 ? home : Option.getOrUndefined(values.userProfile), |
There was a problem hiding this comment.
Fall back to the OS home for Docker contexts
When HOME, USERPROFILE, and DOCKER_CONFIG are absent—for example under a minimal service or cron environment—this leaves home undefined even though the OS account still has a home directory. dockerConfigDir then skips ~/.docker/config.json and the context store entirely, so commands using a non-default Docker context fall back to 127.0.0.1 while Docker operates against the remote daemon; the previous node:os homedir() lookup retained the OS-account fallback. Obtain the home directory from the injected runtime/OS service when neither environment variable is usable.
AGENTS.md reference: apps/cli/AGENTS.md:L52-L53
Useful? React with 👍 / 👎.
Summary
Reviewer context
This PR is stacked on #6303 and intentionally accepts its Effect RPC stack transport as the baseline. Merge #6303 first, then this PR. The stack changes here are lint remediation against that RPC design; they do not restore the prior transport implementation.
Enabling the recommended rules surfaced diagnostics across every TypeScript workspace, so this is intentionally a broad migration. Cleanup was limited to concrete ownership and lifecycle improvements; larger service-threading proposals without a demonstrated runtime benefit were rejected.