feat(lint): require Effect functions for reusable generator wrappers#22000

merged3
effect-fn/clients
effect-fn/enforcement
opened9d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 8d ago

Problem

The earlier Effect function migration can regress because reusable Effect.gen wrappers have no required lint diagnostic and no configured native conversion suggestions. New reusable operations should default to named Effect.fn, with deliberate Effect.fnUntraced exceptions.

Change

Add t3code/prefer-effect-fn to prevent reusable functions from reverting to Effect.gen wrappers. Recognize import aliases, functions, service members, and class methods while allowing inline callbacks, effect-valued getters, and test scaffolding. The diagnostic defaults to named Effect.fn and calls out deliberate fnUntraced exceptions.

Enable the native effectFnOpportunity diagnostic as a suggestion, with existing, inferred, or suggested named-span quick fixes. Its broader matches include inline callbacks and synchronous setup, so the scoped lint rule remains the required check. Exclude test scaffolding from the suggestions.

Tracing choices

  • The required lint rule directs reusable Effect.gen wrappers toward named Effect.fn. The native effectFnOpportunity diagnostic is enabled as a suggestion with effectFn: ["span", "inferred-span", "suggested-span"], so its quick fixes propose named spans.
  • Effect.fnUntraced remains an explicit choice for documented hot paths, long-running loops, or existing-span boundaries, rather than a general alternative. Inline programs, layer construction, and test scaffolding are outside the required wrapper migration; the broader native suggestions remain advisory. This layer changes enforcement and editor guidance, not runtime functions.

Scope and approval

This is the maintainer-requested enforcement layer of the Effect function migration. The maintainer also explicitly asked to enable the native Effect checker suggestions. It adds a required lint rule and advisory editor diagnostics; it does not convert runtime functions in this layer. The work is authored by juliusmarminge (maintainer list); there is no separate public approval discussion.

Depends on #21998.

Verification

The lint-plugin typecheck and all 23 rule tests passed. Targeted lint passed across the changed TypeScript files; existing warnings remain outside this refactor. Shared and server typechecks also pass with the checker configuration enabled; a temporary probe verified that reusable wrappers receive named-span suggestions and inline Effect callbacks remain unchanged.

Nonvisual refactor; evidence comes from the focused behavior and tracing tests. No UI verification applies.

Diff with whitespace ignored

ViewAdded linesRemoved lines
Normal diff3481
Whitespace ignored (git diff -w)3481

Review the files with whitespace hidden. These counts and the patch below are a snapshot of 81d8374, compared with this PR's merge base. Whitespace-only changes are ignored; line wrapping and other textual changes still count.

Expand the full whitespace-ignored patch (5 files; unchanged context omitted)
diff
1diff --git a/oxlint-plugin-t3code/index.ts b/oxlint-plugin-t3code/index.ts
2index f732631246..ede9f4ed94 100644
3--- a/oxlint-plugin-t3code/index.ts
4+++ b/oxlint-plugin-t3code/index.ts
5@@ -21,0 +22 @@ import preferCatchTags from "./rules/prefer-catch-tags.ts";
6+import preferEffectFn from "./rules/prefer-effect-fn.ts";
7@@ -48,0 +50 @@ export default definePlugin({
8+ "prefer-effect-fn": preferEffectFn,
9diff --git a/oxlint-plugin-t3code/rules/prefer-effect-fn.test.ts b/oxlint-plugin-t3code/rules/prefer-effect-fn.test.ts
10new file mode 100644
11index 0000000000..ba48fc6ad5
12--- /dev/null
13+++ b/oxlint-plugin-t3code/rules/prefer-effect-fn.test.ts
14@@ -0,0 +1,202 @@
15+import { assert, describe } from "@effect/vitest";
16+
17+import { createOxlintRuleHarness } from "../test/utils.ts";
18+
19+const rule = createOxlintRuleHarness("t3code/prefer-effect-fn");
20+
21+describe("t3code/prefer-effect-fn", () => {
22+ rule.valid(
23+ "allows inline programs and layer construction",
24+ `
25+ import * as Effect from "effect/Effect";
26+ import * as Layer from "effect/Layer";
27+ export const program = Effect.gen(function* () { yield* Effect.void; });
28+ export const layer = Layer.effectDiscard(Effect.gen(function* () { yield* Effect.void; }));
29+ `,
30+ );
31+
32+ rule.valid(
33+ "allows named functions and deliberate untraced functions",
34+ `
35+ import * as Effect from "effect/Effect";
36+ export const read = Effect.fn("Files.read")(function* () { yield* Effect.void; });
37+ export const chunk = Effect.fnUntraced(function* () { yield* Effect.void; });
38+ `,
39+ );
40+
41+ rule.valid(
42+ "allows inline combinator callbacks",
43+ `
44+ import * as Effect from "effect/Effect";
45+ export const program = Effect.void.pipe(Effect.flatMap(() => Effect.gen(function* () {
46+ yield* Effect.void;
47+ })));
48+ `,
49+ );
50+
51+ rule.valid(
52+ "allows inline option and error callbacks",
53+ `
54+ import * as Effect from "effect/Effect";
55+ import * as Option from "effect/Option";
56+ export const program = Option.match(Option.none(), {
57+ onNone: () => Effect.gen(function* () { yield* Effect.void; }),
58+ onSome: () => Effect.void,
59+ });
60+ export const recovered = Effect.fail({ _tag: "Failed" }).pipe(Effect.catchTags({
61+ Failed: () => Effect.gen(function* () { yield* Effect.void; }),
62+ }));
63+ `,
64+ );
65+
66+ rule.valid(
67+ "allows reference default-value callbacks",
68+ `
69+ import * as Context from "effect/Context";
70+ import * as Effect from "effect/Effect";
71+ export const Default = Context.Reference("Default", {
72+ defaultValue: () => Effect.gen(function* () { yield* Effect.void; }),
73+ });
74+ `,
75+ );
76+
77+ for (const name of ["onSome", "onNone", "onSuccess", "onFailure", "defaultValue"]) {
78+ rule.invalid(
79+ `reports service members named ${name}`,
80+ `
81+ import * as Effect from "effect/Effect";
82+ export const service = Service.of({
83+ ${name}: () => Effect.gen(function* () { yield* Effect.void; }),
84+ });
85+ `,
86+ );
87+ }
88+
89+ rule.valid(
90+ "does not mistake other namespaces or shadowed locals for Effect",
91+ `
92+ import * as Effect from "effect/Effect";
93+ const Other = { gen: (body: () => unknown) => body() };
94+ export const other = () => Other.gen(function* () {});
95+ export function scoped(Effect: typeof Other) {
96+ return Effect.gen(function* () {});
97+ }
98+ export const effect = Effect.void;
99+ `,
100+ );
101+
102+ rule.valid(
103+ "leaves functions with synchronous setup for manual review",
104+ `
105+ import * as Effect from "effect/Effect";
106+ export const read = () => {
107+ const state = new Map();
108+ return Effect.gen(function* () { yield* Effect.void; return state; });
109+ };
110+ `,
111+ );
112+
113+ rule.invalid(
114+ "reports a named arrow wrapper",
115+ `
116+ import * as Effect from "effect/Effect";
117+ export const read = (path: string) => Effect.gen(function* () {
118+ yield* Effect.void;
119+ return path;
120+ });
121+ `,
122+ (output) => {
123+ assert.match(output, /Effect\.fn/);
124+ assert.match(output, /documented hot paths/);
125+ },
126+ );
127+
128+ rule.invalid(
129+ "reports a block-bodied wrapper",
130+ `
131+ import * as Effect from "effect/Effect";
132+ export const read = () => { return Effect.gen(function* () { yield* Effect.void; }); };
133+ `,
134+ );
135+
136+ rule.invalid(
137+ "reports a declaration with pipe transforms",
138+ `
139+ import * as Effect from "effect/Effect";
140+ export function read() {
141+ return Effect.gen(function* () { yield* Effect.void; }).pipe(Effect.asVoid);
142+ }
143+ `,
144+ );
145+
146+ rule.invalid(
147+ "reports a function expression",
148+ `
149+ import * as Effect from "effect/Effect";
150+ export const read = function() { return Effect.gen(function* () { yield* Effect.void; }); };
151+ `,
152+ );
153+
154+ rule.invalid(
155+ "reports service members",
156+ `
157+ import * as Effect from "effect/Effect";
158+ export const service = {
159+ read: () => Effect.gen(function* () { yield* Effect.void; }),
160+ };
161+ `,
162+ );
163+
164+ rule.invalid(
165+ "reports class fields",
166+ `
167+ import * as Effect from "effect/Effect";
168+ export class Service {
169+ read = () => Effect.gen(function* () { yield* Effect.void; });
170+ }
171+ `,
172+ );
173+
174+ rule.invalid(
175+ "recognizes namespace aliases",
176+ `
177+ import * as Eff from "effect/Effect";
178+ export const read = () => Eff.gen(function* () { yield* Eff.void; });
179+ `,
180+ );
181+
182+ rule.invalid(
183+ "recognizes bare-package Effect imports",
184+ `
185+ import { Effect } from "effect";
186+ export const read = () => Effect.gen(function* () { yield* Effect.void; });
187+ `,
188+ );
189+
190+ rule.invalid(
191+ "recognizes a direct gen import",
192+ `
193+ import { gen, void as noop } from "effect/Effect";
194+ export const read = () => gen(function* () { yield* noop; });
195+ `,
196+ );
197+ rule.invalid(
198+ "reports class methods",
199+ `
200+ import * as Effect from "effect/Effect";
201+ export class Service {
202+ read() { return Effect.gen(function* () { yield* Effect.void; }); }
203+ }
204+ `,
205+ );
206+
207+ rule.valid(
208+ "allows getters that expose an effect value",
209+ `
210+ import * as Effect from "effect/Effect";
211+ export class Service {
212+ get read() { return Effect.gen(function* () { yield* Effect.void; }); }
213+ }
214+ `,
215+ );
216+});
217diff --git a/oxlint-plugin-t3code/rules/prefer-effect-fn.ts b/oxlint-plugin-t3code/rules/prefer-effect-fn.ts
218new file mode 100644
219index 0000000000..f72cbb7c2d
220--- /dev/null
221+++ b/oxlint-plugin-t3code/rules/prefer-effect-fn.ts
222@@ -0,0 +1,116 @@
223+import { defineRule, type ESTree } from "@oxlint/plugins";
224+
225+export default defineRule({
226+ meta: {
227+ type: "suggestion",
228+ docs: {
229+ description: "Use Effect.fn for reusable generator-based Effect functions.",
230+ },
231+ },
232+ create(context) {
233+ const isEffectImport = (identifier: ESTree.IdentifierReference, member: string) => {
234+ let scope = context.sourceCode.getScope(identifier);
235+ while (true) {
236+ const variable = scope.set.get(identifier.name);
237+ if (variable !== undefined) {
238+ return variable.defs.some((def) => {
239+ if (def.type !== "ImportBinding" || def.parent?.type !== "ImportDeclaration") {
240+ return false;
241+ }
242+ const source = def.parent.source.value;
243+ if (source === "effect/Effect") {
244+ if (member === "") {
245+ return (
246+ def.node.type === "ImportSpecifier" &&
247+ (def.node.imported.type === "Identifier"
248+ ? def.node.imported.name
249+ : def.node.imported.value) === "gen"
250+ );
251+ }
252+ return def.node.type === "ImportNamespaceSpecifier";
253+ }
254+ return (
255+ source === "effect" &&
256+ def.node.type === "ImportSpecifier" &&
257+ (def.node.imported.type === "Identifier"
258+ ? def.node.imported.name
259+ : def.node.imported.value) === "Effect"
260+ );
261+ });
262+ }
263+ if (scope.upper === null) return false;
264+ scope = scope.upper;
265+ }
266+ };
267+
268+ const isGeneratorCall = (expression: ESTree.Expression): boolean => {
269+ if (expression.type !== "CallExpression") return false;
270+ const callee = expression.callee;
271+ if (callee.type === "Identifier") return isEffectImport(callee, "");
272+ if (callee.type !== "MemberExpression" || callee.computed) return false;
273+ if (callee.property.type !== "Identifier") return false;
274+ if (callee.property.name === "pipe") return isGeneratorCall(callee.object);
275+ return (
276+ callee.property.name === "gen" &&
277+ callee.object.type === "Identifier" &&
278+ isEffectImport(callee.object, "gen")
279+ );
280+ };
281+
282+ const check = (node: ESTree.ArrowFunctionExpression | ESTree.Function) => {
283+ if (node.async || node.generator) return;
284+ const parent = node.parent;
285+ if (node.type !== "FunctionDeclaration") {
286+ if (parent.type === "MethodDefinition") {
287+ if (parent.kind !== "method") return;
288+ } else if (parent.type === "Property") {
289+ if (parent.kind !== "init") return;
290+ const name =
291+ parent.key.type === "Identifier"
292+ ? parent.key.name
293+ : parent.key.type === "Literal"
294+ ? parent.key.value
295+ : undefined;
296+ const call = parent.parent.parent;
297+ if (call?.type === "CallExpression") {
298+ let callee = call.callee;
299+ while (callee.type === "CallExpression") callee = callee.callee;
300+ if (callee.type === "MemberExpression" && callee.property.type === "Identifier") {
301+ if (
302+ ["catchTags", "match", "matchEffect", "matchCauseEffect"].includes(
303+ callee.property.name,
304+ ) ||
305+ (name === "defaultValue" && callee.property.name === "Reference")
306+ ) {
307+ return;
308+ }
309+ }
310+ }
311+ } else if (parent.type !== "VariableDeclarator" && parent.type !== "PropertyDefinition") {
312+ return;
313+ }
314+ }
315+ const body = node.body;
316+ if (body === null) return;
317+ let expression: ESTree.Expression | null;
318+ if (body.type === "BlockStatement") {
319+ if (body.body.length !== 1 || body.body[0]?.type !== "ReturnStatement") return;
320+ expression = body.body[0].argument;
321+ } else {
322+ expression = body;
323+ }
324+ if (expression === null || !isGeneratorCall(expression)) return;
325+ context.report({
326+ node,
327+ message:
328+ 'Use Effect.fn("Service.operation") for reusable Effect functions. Reserve Effect.fnUntraced for documented hot paths or an existing span; keep Effect.gen for inline programs.',
329+ });
330+ };
331+
332+ return {
333+ ArrowFunctionExpression: check,
334+ FunctionExpression: check,
335+ FunctionDeclaration: check,
336+ };
337+ },
338+});
339diff --git a/tsconfig.base.json b/tsconfig.base.json
340index 17efedde93..07c4224cf8 100644
341--- a/tsconfig.base.json
342+++ b/tsconfig.base.json
343@@ -24,0 +25 @@
344+ "effectFn": ["span", "inferred-span", "suggested-span"],
345@@ -25,0 +27 @@
346+ "effectFnOpportunity": "suggestion",
347@@ -62 +64,17 @@
348- "options": { "diagnosticSeverity": { "preferSchemaOverJson": "off" } }
349+ "options": {
350+ "diagnosticSeverity": {
351+ "preferSchemaOverJson": "off",
352+ "effectFnOpportunity": "off"
353+ }
354+ }
355+ },
356+ {
357+ "include": [
358+ "**/*.spec.ts",
359+ "**/*.spec.tsx",
360+ "**/*.testkit.ts",
361+ "**/*.testkit.tsx",
362+ "**/{test,tests,testkit,testing,fixtures,integration}/**",
363+ "packages/provider-testing/**"
364+ ],
365+ "options": { "diagnosticSeverity": { "effectFnOpportunity": "off" } }
366diff --git a/vite.config.ts b/vite.config.ts
367index e2b3416d88..8866c952a5 100644
368--- a/vite.config.ts
369+++ b/vite.config.ts
370@@ -174,0 +175 @@ export default defineConfig({
371+ "t3code/prefer-effect-fn": "error",
372@@ -178,0 +180,8 @@ export default defineConfig({
373+ {
374+ files: [
375+ "**/*.{test,spec,testkit}.{ts,tsx}",
376+ "**/{test,tests,testkit,testing,fixtures,integration}/**",
377+ "packages/provider-testing/**",
378+ ],
379+ rules: { "t3code/prefer-effect-fn": "off" },
380+ },

Model: GPT-6.1 Sol. Harness: Codex.

Checks

23 successful · 7 skipped
All checks passed
7ac407a
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—4.9 KiB—6.8 KiB✅
CodexThread snapshot wire—3.8 KiB—4.9 KiB✅
CodexLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
CodexLive turn WebSocket decoded—20.8 KiB—29.3 KiB✅
CodexLive turn messages—1—8✅
ClaudeTotal thread wire—5.0 KiB—6.8 KiB✅
ClaudeThread snapshot wire—3.8 KiB—4.9 KiB✅
ClaudeLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
ClaudeLive turn WebSocket decoded—21.2 KiB—29.3 KiB✅
ClaudeLive turn messages—2—8✅

Baseline: unavailable · PR result: 7ac407a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

oxlint-plugin-t3code/rules/prefer-effect-fn.tsline 76Outdated · 683bafeOpen thread
……@@ -0,0 +1,115 @@
73+ : parent.key.type === "Literal"
74+ ? parent.key.value
75+ : undefined;
76+ if (typeof name === "string" && CALLBACK_NAMES.has(name)) return;
MmacroscopeappBoton line 769d ago

🟠 High rules/prefer-effect-fn.ts:76

A reusable service method named onSuccess, onFailure, or defaultValue receives no diagnostic, even when its object is not used as callback options. The name check at line 76 skips these properties before their context is verified; restrict the exemption to callback-option objects.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @oxlint-plugin-t3code/rules/prefer-effect-fn.ts around line 76:
2
3A reusable service method named `onSuccess`, `onFailure`, or `defaultValue` receives no diagnostic, even when its object is not used as callback options. The name check at line 76 skips these properties before their context is verified; restrict the exemption to callback-option objects.

Fixed in 7d01a7c6a1: callback exemptions now require the matching option-object context. Added coverage for all five callback-like names on ordinary service members and for reference defaults; all 23 rule tests and the plugin typecheck pass.

Verified: ordinary service members with all five names are now reported, while exemptions are scoped to the relevant option objects.

Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR adds a new Effect wrapper lint rule and enables related language-service diagnostics, with no application runtime changes. It also adds broad test and fixture exclusions that suppress the new static-analysis diagnostics, so the configuration changes require human review.

No code changes detected at 593d8e4. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

Synced from GitHub

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 4cf32085-1740-4afc-8829-7da1bd62e392
📥 Commits

Reviewing files that changed from the base of the PR and between 7a6b914395d5329d63dae1a70baa2c01402afa34 and 7ac407a41f8c292efa5af14d1737a58f3c94e41c.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: c7f3c690-0125-4752-8d3e-c34382f36a2c
📥 Commits

Reviewing files that changed from the base of the PR and between a571abf250d026fac04f333018e02eaf2a82f65d and 2e01e6b1420ee7cb168816358ab31ce372a2a7df.

📒 Files selected for processing: 5
  • oxlint-plugin-t3code/index.ts
  • oxlint-plugin-t3code/rules/prefer-effect-fn.test.ts
  • oxlint-plugin-t3code/rules/prefer-effect-fn.ts
  • tsconfig.base.json
  • vite.config.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

The plugin adds a rule that reports eligible non-async, non-generator functions returning Effect.gen calls. The configuration enables related language-service suggestions and disables the rule or suggestion for specified test-related paths.

Changes: Effect generator function lint rule

Layer / File(s)Summary
Detect and test Effect generator functions
oxlint-plugin-t3code/rules/prefer-effect-fn.ts, oxlint-plugin-t3code/rules/prefer-effect-fn.test.ts
Adds import-aware detection and diagnostics for eligible functions returning generator calls. Tests cover reported patterns, accepted patterns, and diagnostic text.
Register and configure the rule
oxlint-plugin-t3code/index.ts, tsconfig.base.json, vite.config.ts
Registers the rule in the plugin. Configures Effect function suggestions and disables related diagnostics or the rule for specified test-related paths.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Suggested reviewers: esthor

Merge Risk: ⚪ Minimal · up to 2e01e

No actionable issue currently blocks merging. The rule is configured for source files and excluded from test scaffolding; normal checks remain appropriate.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title is concise, specific, and accurately summarizes the main change: requiring reusable generator wrappers to use Effect functions.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Description checkPassedThe pull request description follows the required template. It explains the problem, change, scope and approval basis, verification steps, limitations, and nonvisual status. It also identifies the dep…
Full details: Description check

Explanation

The pull request description follows the required template. It explains the problem, change, scope and approval basis, verification steps, limitations, and nonvisual status. It also identifies the dependency, test results, remaining warnings, and the model and harness used.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Rebased the full stack onto fb99076 for the maintainer-authorized landing. Resolved seven overlapping files while preserving the newer model-selection, rollback-context, startup-guard, delegated-task deduplication/result, and VCS refresh behavior from main. The fn/fnUntraced boundaries remain intact.

Validation after refreshing worktree dependencies: 190 focused server tests plus 37 provider tests passed; server, ACP, and Grok typechecks passed. Whitespace-ignored diff snapshots were refreshed for all twelve descriptions. Waiting for current-head CI before the native whole-stack merge.

Added 3 commits

  • fix(lint): limit Effect callback exemptions to option objects7ac407a
  • chore(effect): enable named function checker suggestionsbdac933
  • feat(lint): require Effect functions for reusable generator wrappersf2a3a61
Synced from GitHub

Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting).

This review would cost an estimated $16.20, which exceeds your per-review limit of $15.00.

The top 3 files driving up this estimate:

FileDiff SizeEstimate
apps/server/src/orchestration-v2/Orchestrator.ts403.70KB$3.91
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts118.66KB$1.15
apps/server/src/mcp/OrchestratorMcpService.ts109.25KB$1.06

Tip

To get this pull request reviewed, you can:

  1. Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
  2. Exclude the file(s) above from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
  3. Raise your cost limit in your workspace billing settings.

Turn off this reminder going forward