Skip to content

A class DECLARATION inside a function is shared across invocations — every member sees the LAST call's captures (class expressions are correct); breaks all Effect Context.Service DI #10835

Description

@proggeramlug

Summary

A static get accessor declared on a class inside a function captures the enclosing function's parameters from the last invocation, not from the call that produced that class. Every class the factory returns shares one closure environment.

A plain static field on the same class is correct, which makes this easy to miss: the class looks right until you read the accessor.

Reproducer

function serviceBase(id: string) {
  return class {
    static key = id
  }
}

function makeService(id: string, fields: Record<string, any>) {
  class ConfigTag extends serviceBase(id) {
    static fields = fields
    static get layer() { return { for: id } }   // <- closes over `id`
  }
  return ConfigTag
}

class Auth  extends makeService("tag-Auth",  { password: 1 })  {}
class Flags extends makeService("tag-Flags", { autoShare: 2 }) {}

console.log("Auth.key   =", Auth.key)
console.log("Auth.fields=", JSON.stringify(Auth.fields))
console.log("Auth.layer =", JSON.stringify(Auth.layer))

bun 1.3.14 / node:

Auth.key   = tag-Auth
Auth.fields= {"password":1}
Auth.layer = {"for":"tag-Auth"}

perry (main + the #10399 branch, macOS arm64, 7 MB binary):

Auth.key   = tag-Auth          <- static FIELD: correct
Auth.fields= {"password":1}    <- static FIELD: correct
Auth.layer = {"for":"tag-Flags"}   <- static GETTER: WRONG, sees the LAST call's `id`

Both Auth and Flags report {"for":"tag-Flags"}. Declaration order decides the winner, so whichever factory call runs last wins for every class.

Why it matters — it silently breaks Effect's whole service/DI layer

This is the OpenCode TUI blocker, traced end to end.

effect's Context.Service pattern is exactly this shape. OpenCode wraps it in src/effect/config-service.ts:

class ConfigTag extends Context.Service<Self, Shape<Fields>>()(id) {
  static get layer() {
    const tag = this
    return Layer.effect(tag, Effect.gen(function* () {
      const config = yield* Config.all(fields)   // <- `fields` captured by the getter
      return tag.of(config as Shape<Fields>)
    }))
  }
}

Exactly two services are built through it — server/auth.ts:17 (@opencode/ServerAuthConfig) and effect/runtime-flags.ts:16 (@opencode/RuntimeFlags). Because layer is a static getter closing over fields and id, both layers provide RuntimeFlags' shape, so the Context map ends up with one entry:

              perry                         bun
A.key     "@opencode/RuntimeFlags"     "@opencode/ServerAuthConfig"
B.key     "@opencode/RuntimeFlags"     "@opencode/RuntimeFlags"
entries    1                            2
keys       ["@opencode/RuntimeFlags"]   ["@opencode/ServerAuthConfig","@opencode/RuntimeFlags"]

Downstream, yield* ServerAuth.Config hands the authorization middleware the flags object:

AUTHPROBE required(): keys=["autoShare","pure","disableDefaultPlugins",...,"client"]
                      password_t=undefined username=null

so auth.ts:24's Option.isSome(config.password) reads undefined._tag and throws. lldb on a PERRY_KEEP_SYMBOLS=1 build:

#0 js_throw_type_error_property_access
#1 js_object_get_field_ic_nonptr
#2 effect/src/internal/option.ts closure #20
#3 opencode/src/server/auth.ts required()
#4 httpapi/middleware/authorization.ts closure #48

The server therefore never starts, which is one cause behind three symptoms: the TUI paints nothing (bootstrap()'s SDK calls never settle, so <Show when={init.ready}> withholds every provider), opencode run reports "Session not found", and the run CLI rung fails.

Ruled out, with measurements

  • Not class-id collision ([[per-module class ids collide]] was the obvious suspect): tag identity is correct — A !== B, distinct .key strings when read directly, different prototypes, different .layer objects.
  • Not Map/string-key hashing: new Map() and plain objects keyed by those exact two strings behave identically to bun.
  • Not Config.all or the service factory: a single-service probe (yield* ServerAuth.Config with only its own layer) returns the correct object under perry.
  • Not a plain static field: static key = id is correct in the same class. Only the accessor is wrong.
  • Not one level of nesting: a single-level factory returning class { static key = id } is correct. Two levels plus a static getter is what reproduces.

Function.prototype.length also returns 0 under perry where bun returns the declared arity (function raw(a,b,c){} → raw.length is 0 vs 3). Effect's dual dispatches on arguments.length (correct here) rather than fn.length, so it is not this bug — filing separately is probably worthwhile.

Activity

  1. proggeramlug commented on Sep 20, 2026

    @proggeramlug
    ContributorAuthor

    Correcting the title and scope: it is not static getters, it is class declarations inside functions

    My original report blamed static get. That was too narrow — it was an artifact of the first reproducer, where the static fields happened to come from an inner class expression. The real rule is sharper and much worse.

    A class DECLARATION inside a function is emitted once and shared across invocations. Every member — static field, static method, static getter, instance method, instance getter — sees the LAST call's captured values. A class EXPRESSION is correct.

    function makeDecl(id: string) {          // class DECLARATION
      class Tag { static key = id; m() { return id } }
      return Tag
    }
    function makeExpr(id: string) {          // class EXPRESSION
      return class { static key = id; m() { return id } }
    }
    function makeConst(id: string) {         // class expression bound to a const
      const Tag = class { static key = id; m() { return id } }
      return Tag
    }

    perry (main + #10399 branch, macOS arm64, 7 MB binary, ~1 min compile):

    decl static            1=D2  2=D2   <-- WRONG
    decl instance          1=D2  2=D2   <-- WRONG
    expr static            1=E1  2=E2   OK
    expr instance          1=E1  2=E2   OK
    const-expr static      1=C1  2=C2   OK
    const-expr instance    1=C1  2=C2   OK
    

    bun 1.3.14 and node return D1/D2 for every row.

    Member-kind matrix on a single-level declaration factory — all five wrong, so this is not accessor-specific:

    static field     A=B  B=B <-- WRONG
    static method    A=B  B=B <-- WRONG
    static getter    A=B  B=B <-- WRONG
    instance method  A=B  B=B <-- WRONG
    instance getter  A=B  B=B <-- WRONG
    

    Why this framing matters for the fix

    perry already implements the correct behaviour for class expressions. const Tag = class {...} produces a fresh class per invocation with correctly captured environment; only the declaration path is wrong. That suggests the fix is to lower a function-local class X {} the way const X = class {} is already lowered, rather than to build new machinery.

    Still the OpenCode blocker

    src/effect/config-service.ts uses exactly this shape:

    export const Service = <Self>() => <Id extends string, Fields extends ConfigMap>(id: Id, fields: Fields) => {
      class ConfigTag extends Context.Service<Self, Shape<Fields>>()(id) {   // <- DECLARATION in a function
        static get layer() { /* captures `fields` and `id` */ }
      }
      return ConfigTag
    }

    so both services built through it (@opencode/ServerAuthConfig, @opencode/RuntimeFlags) collapse onto the last one's captures. The Context map ends up with one entry, yield* ServerAuth.Config returns the flags object, and Option.isSome(config.password) reads undefined._tag. The server never starts — blank TUI, run "Session not found", CLI run rung.

    Reproducer saved locally at perry-repros/issue-10835-static-getter-factory.ts; the declaration-vs-expression matrix above is the better one to pin in a regression test.

  2. changed the title [-]A `static get` in a class returned by a factory captures the LAST call's arguments — breaks every Effect `Context.Service` (OpenCode TUI blocker)[/-] [+]A class DECLARATION inside a function is shared across invocations — every member sees the LAST call's captures (class expressions are correct); breaks all Effect Context.Service DI[/+] on Sep 20, 2026
  3. proggeramlug commented on Sep 20, 2026

    @proggeramlug
    ContributorAuthor

    Mechanism, located — two sub-bugs, both in lower_decl/body_stmt.rs

    A function-nested class declaration takes one of two paths, chosen by fresh_binding (crates/perry-hir/src/lower_decl/body_stmt.rs:388):

    let has_static_state = !class.static_fields.is_empty()
        || class.static_methods.iter().any(|m| m.name.starts_with("__perry_static_init_"));
    let fresh_binding = has_private_elements
        || class.extends_expr.is_some()
        || !computed_keys.is_empty()
        || (!captured_exprs.is_empty() && !has_static_state);

    false → the shared-template path, whose captures go through RegisterClassCaptures, keyed by class_name. The comment right above it already says these registries are "last-wins". true → ClassExprFresh, keyed by the heap class value, which is why class expressions are correct.

    Sub-bug 1 — the has_static_state carve-out

    (!captured_exprs.is_empty() && !has_static_state) means a captured class that also has public static state deliberately stays on the shared template ("Capture-only classes with public static state retain the shared-template path unless another condition requires a fresh evaluation"). That is backwards: static state makes a per-evaluation class more necessary, since each invocation needs its own static values.

    This is what the minimal matrix hits — class Tag { static key = id; m() { return id } } with no extends, so every member reports the last call's id.

    Sub-bug 2 — ClassExprFresh drops static methods and accessors

    This is the one that breaks OpenCode, and it bites even when fresh_binding is already true. lower/stmt.rs:119 names it outright — the "static-method-dropping ClassExprFresh path". ClassExprFresh carries named_statics, computed_keys, computed_statics, static_init_order and captured_args, but no static methods or accessors; those stay on the shared template with whatever captures were registered last.

    OpenCode's ConfigTag has extends Context.Service<Self, Shape<Fields>>()(id), so it already takes the fresh path — and the split is visible in the reproducer:

    Auth.key    = tag-Auth              <- static field, fresh:   correct
    Auth.fields = {"password":1}        <- static field, fresh:   correct
    Auth.layer  = {"for":"tag-Flags"}   <- static GETTER, template: WRONG
    

    So static get layer() — which closes over fields and id — resolves through the shared template and returns the last factory call's environment. Hence both Effect layers provide the same service.

    What a fix has to do

    Carry static methods and accessors per evaluation on ClassExprFresh (bound to that evaluation's capture array), and drop the has_static_state exclusion from fresh_binding. The first is the substantive change; the second is a one-line gate widening that is untestable on its own while the first is outstanding.

    Both sub-bugs are the same family as #9847/#9857 (bare X = native.method() registered X's NAME module-wide, fixed by LocalId keying): registration keyed by a shared name rather than by the evaluated value.

    Reproducers

    decl static            1=D2  2=D2   <-- WRONG   (sub-bug 1)
    decl instance          1=D2  2=D2   <-- WRONG   (sub-bug 1)
    expr static            1=E1  2=E2   OK
    const-expr static      1=C1  2=C2   OK
    

    and, with extends present so fresh_binding is already true, sub-bug 2 alone:

    Auth.key/fields  correct        Auth.layer (static getter)  WRONG
    

    Both compile to ~7 MB binaries in about a minute — far cheaper than the 2h15m OpenCode build.

  4. proggeramlug commented on Sep 20, 2026

    @proggeramlug
    ContributorAuthor

    ⚠ Do not "fix" the fresh_binding gate on its own — the carve-out is load-bearing

    Following up on my own suggestion in the previous comment, because I was about to make this mistake and the measurement stopped me.

    The obvious first move is to drop !has_static_state from fresh_binding (sub-bug 1). That would make things worse, not better. ClassExprFresh is the static-method-dropping path, so widening the gate pushes more classes onto a path that discards their static methods and accessors. Classes that capture outer locals AND have static methods currently work — badly for their captures, but their statics resolve — and they would start losing the statics outright.

    In other words the carve-out is a deliberate mitigation for sub-bug 2, not an oversight. Sub-bug 2 has to be fixed first; the gate widening is only safe afterwards.

    Why sub-bug 2 needs a new channel rather than a rewire

    crates/perry-codegen/src/codegen/method_static.rs contains zero references to __perry_cap_*. Static methods have no capture channel at all:

    • An instance method reaches captures through the synthesized __perry_cap_<id> constructor params stored on the instance, so this.__perry_cap_* is available.
    • A static method binds this to the class ref (method_static.rs:77), and there is no per-evaluation capture storage hanging off that class value for it to read.

    ClassExprFresh already allocates a per-evaluation heap class object carrying captured_args, so the values exist on the right object — the missing piece is (a) storing them at a known slot on that class object and (b) teaching compile_static_method to resolve captured locals through the receiver instead of the module-level template.

    Until that exists, every route is blocked:

    • shared template → captures are name-keyed and last-wins (sub-bug 1)
    • ClassExprFresh → statics are dropped (sub-bug 2)

    which is exactly why OpenCode shows the split result (Auth.key/Auth.fields correct from the fresh path, Auth.layer wrong from the template).

    Suggested order

    1. Give ClassExprFresh class objects a capture slot and have compile_static_method read captured locals from the receiver.
    2. Carry static methods/accessors per evaluation instead of dropping them.
    3. Only then remove !has_static_state from fresh_binding, and pin the declaration-vs-expression matrix as a regression test.

    Each step is independently testable against the ~1-minute reproducers rather than the 2h15m OpenCode build.

  5. proggeramlug commented on Sep 20, 2026

    @proggeramlug
    ContributorAuthor

    The surgical fix does not exist — measured, and the reason is mechanical

    I tried the obvious targeted fix and it failed. Recording why, because it rules out a whole family of attempts.

    Attempt: arm the one-shot static-this override with the real receiver before invoking a static accessor, so the body's js_static_this_resolve yields the per-evaluation class object rather than the template class-ref, letting js_class_capture_value_for_receiver find that object's __perry_ctor_caps.

    Result: no change — Auth.layer still returns the last factory call's value.

    Why, from lldb on a PERRY_KEEP_SYMBOLS=1 build of the reproducer:

    frame #1  perry_static_two_ts__ConfigTag__c5____get_layer
    frame #2  perry_runtime::object::class_registry::parent_static::class_static_accessor_getter_value
    frame #3  js_object_get_field_by_name
    

    Two things that matter:

    1. Static accessors dispatch through class_static_accessor_getter_value, not the class_dynamic_static_accessor_getter_value path (which is where the existing receiver-aware capture read lives).
    2. js_class_capture_value and js_class_capture_value_for_receiver are reported by lldb as "no locations (pending)" — neither symbol is in the binary. The static accessor body never consults the capture API at all.

    Disassembly of the getter shows what it does instead:

    adrp x9, 0x10078c000        ; fixed module-level global
    ldr  w19, [x9, #0xd84]      ; <- reads the captured `id` from there
    bl   _js_static_this_resolve

    The captured outer local was promoted to a module-level global slot. Every invocation of the enclosing factory overwrites it, so the getter reads whatever the last call stored. No amount of receiver plumbing helps, because the body is not reading a receiver-relative location at all.

    What this means for the fix

    The capture machinery (__perry_ctor_caps on the class object, js_class_capture_value_for_receiver, the per-evaluation/decl-site/undefined fallback chain) is built and works — for constructors and for the paths that call it. Static methods and accessors are simply outside it: crates/perry-codegen/src/codegen/method_static.rs contains zero references to __perry_cap_*.

    So the work is in lowering/codegen, not in dispatch:

    1. Include static method and accessor bodies in synthesize_class_captures, so their references to enclosing locals are recognised as captures instead of being promoted to module globals.
    2. Emit a capture-rebind prologue in compile_static_method that resolves each captured local through the receiver (js_class_capture_value_for_receiver already implements the right fallback order).
    3. Carry static methods/accessors per evaluation on ClassExprFresh.
    4. Only then drop !has_static_state from fresh_binding.

    Steps 1–2 are the substantive ones and are independently testable against the ~1-minute reproducer.

  6. proggeramlug commented on Sep 20, 2026

    @proggeramlug
    ContributorAuthor

    Complete fix specification — the divergence is two loops in class_captures.rs

    crates/perry-hir/src/lower_decl/class_captures.rs rewrites each member's references to enclosing-function locals. Two strategies exist, and members are routed by which vector they live in, not by whether they are static:

    for (_, g) in getters.iter_mut() {           // line 302 — ALL getters, including STATIC ones
        let id_map = rewrite_method_body(ctx, &mut g.body);   // instance strategy: this.__perry_cap_*
        append_self_sites(&mut g.body, &id_map);
    }
    ...
    for sm in static_methods.iter_mut() {        // line 325 — decl-site snapshot strategy

    Static accessors are stored in getters/setters (flagged separately by static_accessor_fn_ids), so they receive the instance rewrite. A static accessor has no instance — this is the class-ref — so this.__perry_cap_* resolves to nothing and the reference falls back to the enclosing local, which lowering has promoted to a module-level global. That is exactly what the disassembly shows:

    adrp x9, 0x10078c000
    ldr  w19, [x9, #0xd84]     ; captured `id`, read from a module-level slot

    One global slot, overwritten by every factory invocation.

    Both static strategies are wrong, in different ways

    The member-kind matrix on a single-level declaration factory shows all five broken, which rules out "accessors only":

    static field     A=B  B=B <-- WRONG
    static method    A=B  B=B <-- WRONG
    static getter    A=B  B=B <-- WRONG
    instance method  A=B  B=B <-- WRONG
    instance getter  A=B  B=B <-- WRONG
    
    • Static accessors: mis-routed to the instance strategy → promoted module global → last-wins.
    • Static methods: correctly routed, but the decl-site snapshot (ClassCaptureValue → js_class_capture_value(class_id, index)) is keyed by class_id, shared by every evaluation of the template → also last-wins. RegisterClassCaptures is name-keyed and the code comments already call it "last-definition-wins".

    So routing static accessors to the existing static-method strategy would make them consistently wrong rather than correct.

    What a correct fix requires — three coordinated changes

    The per-evaluation machinery already exists and is used by constructor replay; static members are simply outside it.

    1. HIR (class_captures.rs): route members by static-ness rather than by vector, and have static members emit a receiver-aware capture read rather than ClassCaptureValue.
    2. Codegen (codegen/method_static.rs, currently zero references to __perry_cap_*): lower that to js_class_capture_value_for_receiver(this, class_id, index), which already implements the right precedence — per-evaluation __perry_ctor_caps first, decl-site snapshot second, undefined last.
    3. Runtime (class_registry/parent_static): arm the one-shot static-this override with the real receiver in class_static_accessor_getter_value (and its setter twin) so step 2's this is the evaluated class object rather than the template class-ref.

    I verified step 3 alone is insufficient — I implemented it, measured no change, and reverted it. lldb showed why: the accessor body never calls the capture API at all, so there is nothing for the armed receiver to feed. The three changes only work together.

    Afterwards, !has_static_state can come out of fresh_binding, and the declaration-vs-expression matrix plus the member-kind matrix should both be pinned as regression tests.

  7. proggeramlug commented on Sep 23, 2026

    @proggeramlug
    ContributorAuthor

    Fixed on main by 796d6a5726 ("fix(hir): a static accessor reads its own receiver's captures"), which landed in #10859 as v0.5.1635. Regression test on main: crates/perry/tests/issue_10835_static_accessor_captures.rs.

    A static get/static set on a class that closed over its factory's arguments read the capture from the class's declaration-site slot, keyed by class NAME, so the last evaluation won and every earlier class from the same factory answered with the last one's values — the Effect Context.Service shape described here. #10859 verified it both ways: the new test passes with the fix and fails without it with exactly that diff.

    It stayed open because #10859 described this fix in prose without a close keyword (its only keyword was Fixes #10399), so GitHub never linked it. Closing by hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions