Skip to content

executor-requires-description forgives unmigrated workflow calls in any file that declares one description #158

Description

@younna-ai-opencode

labels:
priority: p1
type: bug
area: tooling
metadata:
rule: executor-requires-description
pinned_commit: 1574bfa
relationship: "#139 owns the message half; this is the check half #139 says needs its own issue"

Problem Statement

A new lint rule is supposed to reject any call site that reaches a workflow decision without expressing it as a Cell description. It does not check call sites -- it checks files. If a Cell method appears anywhere in the file, every bare workflow call in that file is forgiven. A half-migrated file, one description plus one hand-sequenced call left behind, passes a deny-severity gate, which is exactly the shape the rule exists to catch. The rule's own test suite pins that pass as correct.

Goal

executor-requires-description reports every call to a workflow-module decision that is not lexically inside a Cell phase argument, in every non-test file it runs on, regardless of whether the same file contains other Cell descriptions -- and no file containing both a description and a bare workflow decision call passes the rule.

Evidence: the gate is file-scoped

        if (cellBindings.has(root) && DESCRIPTION_METHODS.some((method) => method === property.name)) {
          declaresDescription = true
        }
      },
      'Program:exit'() {
        if (declaresDescription) return
        for (const { node, name } of workflowCalls) {
          context.report({

Source: packages/oxlint-plugins/effect-executor/src/rules/executor-requires-description.ts:76-83 at 1574bfaa3c95d37c08475aff3806d5cf084c3dd0

declaresDescription is one file-wide boolean, declared at line 40. Any one of read/decode/decide/encode/write/apply sets it -- an incomplete Cell.read with no chain and no Cell.apply is enough -- and Program:exit then discards every call collected.

Evidence: a passing fixture is the violation

      name: 'Should_Pass_When_WorkflowCallAndDescriptionDeclaredLaterInFile',
      code: `${WORKFLOW_IMPORT}
import { Cell } from '@systemfsoftware/effect-cell-types'

export const run = (input: unknown): unknown => {
  const outcome = decide(input)
  const description = Cell.write(() => Effect.succeed(outcome))
  return Cell.apply(description, input)
}
`,

Source: packages/oxlint-plugins/effect-executor/src/rules/__tests__/executor-requires-description.test.ts:157-166 at 1574bfaa3c95d37c08475aff3806d5cf084c3dd0

decide(input) is a bare workflow decision outside any description, and this fixture sits in the valid array.

Orientation

The code cited here lands via branch feat/phase-order-as-description and is not on main yet. Every line reference is pinned to commit 1574bfaa3c95d37c08475aff3806d5cf084c3dd0; read it there.

  • Rule: packages/oxlint-plugins/effect-executor/src/rules/executor-requires-description.ts. Message constants: the sibling .config.ts. Fixtures: src/rules/__tests__/.
  • The sibling rule executor-no-io-in-filling.ts already walks into phase-constructor arguments to inspect their bodies. That is the traversal a per-call-site check needs; match its shape rather than inventing one.
  • meta.docs currently describes the honest per-file behaviour while REQUIRES_DESCRIPTION_EXPECTED claims "every call site". Whichever direction this resolves, those strings and meta.docs must agree at the end.
  • Related: Rule messages may assert properties their single-file checks cannot decide #139 owns rewording rule messages repo-wide and explicitly excludes changing a rule's check, stating that widening a check "needs its own issue". This is that issue. Do not reword other rules' messages here.
  • packages/oxlint-plugins/effect-executor/AGENTS.md EE1 (classify by import edge, never by a name's spelling) and EE2 (no rule ships that fires on sanctioned code) both bind this work.

Definitions

  • Inside a description -- the call expression is a descendant of an argument passed to a Cell.<phase> call whose Cell binding is imported from @systemfsoftware/effect-cell-types. Sharing a file is not being inside.
  • Bare workflow decision call -- a call whose callee root resolves to a binding or namespace imported from a module cellOf classifies as workflow, and which is not inside a description.
  • Degenerate cases: a file with a description and no workflow calls (clean); a file with a workflow call and no Cell import at all (reported today, must stay reported); a workflow call inside a Cell.decide body (clean); a workflow call in the same file as, but outside, a complete description (clean today, must be reported); a lone Cell.read with no other phase and no apply (must forgive nothing).

Non-Counting Outcomes

  • Reporting the second call only when it appears before the description, or otherwise keying the verdict on source position. Position is not the property; containment is.
  • Deleting or renaming Should_Pass_When_WorkflowCallAndDescriptionDeclaredLaterInFile to make the suite green without deciding what that shape should do. It ends in invalid with the mixed-file behaviour asserted, or stays valid with a stated reason.
  • Narrowing the rule's message and leaving the predicate file-scoped. That is Rule messages may assert properties their single-file checks cannot decide #139's work, and shipping it here closes this issue without changing any behaviour.
  • Making the rule fire on legitimate code to force the strict reading. A new false positive on any real *.executor.ts breaches EE2 and does not count.
  • Widening DESCRIPTION_METHODS, cellBindings, or the import edge so that more files trip the flag, instead of attributing calls to descriptions.
  • A survey of affected call sites, or a proposal for per-call-site attribution, instead of a rule that reports them.

Acceptance Criteria

  • Critical, gatekeeper. pnpm --filter @systemfsoftware/oxlint-plugin-effect-executor test exits 0, and the suite contains an invalid fixture for a file holding one complete description plus one bare workflow decision call, asserting a report on the bare call.
  • Critical. Reverting the predicate change alone makes that new fixture fail. A green run with the old predicate in place does not satisfy this issue.
  • A file whose only Cell usage is a single Cell.read with no other phase and no Cell.apply does not suppress a report on a bare workflow decision call in that file, and a fixture pins it.
  • Every degenerate case named in Definitions has an explicit fixture and an asserted outcome.
  • pnpm check:local exits 0 after the last edit, with no new diagnostic on any existing *.executor.ts in the repo.
  • REQUIRES_DESCRIPTION_EXPECTED, REQUIRES_DESCRIPTION_ACTUAL and meta.docs describe the same granularity the shipped predicate decides.
  • packages/oxlint-plugins/effect-executor/ carries a .changeset/ intent.

Verification

An auditor must confirm each of these against the diff and a clean checkout:

  1. Red-green proof. The new mixed-file fixture fails with the predicate reverted and passes with it applied. One green run proves nothing.
  2. Circularity analogue. The fixture's expected reports must not be produced by running the rule and recording what it emitted -- an expectation derived from the implementation's own output asserts the implementation against itself and would pass for any predicate, including the current one. Expected reports are written from the intended contract.
  3. No green-by-deletion. No fixture removed, renamed to dodge, silently moved to valid, or weakened; no .skip/.only; no assertion count loosened.
  4. The subject is not mocked away. The suite exercises the real rule through RuleTester, not a stand-in for the traversal.
  5. No check disabled for green. No severity lowered, no configs.recommended membership dropped, no lint or type check disabled, no @ts-expect-error added to pass.
  6. Clean checkout. The suite passes from a fresh clone at the branch head with a cold cache, not only in a workspace holding an earlier build.
  7. Whole diff committed. No untracked or unstaged file carries part of the change.
  8. No documentation lie. The message strings and meta.docs do not claim a granularity the shipped predicate does not decide. Confirm by reading the predicate, not the prose.

Return Condition

Return done only when every acceptance criterion and every verification item holds, citing runnable evidence: the command, its exit code, and the fixture names that changed. If the work stops before that, return incomplete and name exactly which degenerate cases remain unpinned and whether the predicate changed at all. A message-only edit reported as progress on this issue is a near miss, not partial success.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions