fix(import): isolate module top-level bare assignments in module scope (fixes #589)

docs/SPEC.md

@@ -1032,8 +1032,15 @@ can read the loader's globals and call its functions, but they can
 never bind a write through to them — a bare `name is expr` inside a
 module function that doesn't refer to a local, a captured name, or the
 module's own top-level state creates a fresh local, regardless of what
-happens to exist in the loader's scope. (Top-level statements in the
-loaded file still execute directly in the current scope.) To share
+happens to exist in the loader's scope. The same boundary applies one
+level up to an **imported** module's own top-level statements: a bare
+`name is expr` at an imported file's top level binds in *that module's
+own scope*, never walking through to a same-named binding the importer
+happens to already have — `counter is 0` at a module's top level can
+never rebind an importer's pre-existing `counter`. `load_file` is the
+one exception, per its older, documented contract above: its top-level
+statements still execute directly in the current (caller's) scope, so
+a same-named top-level assignment there *does* bind through. To share
 mutable state across files, put it in a dict or list and mutate fields
 — reads cross the boundary and field/index writes are value mutations,
 not bindings. The standard library's UI toolkit (`lib/ui.eigs`'s `_ui`

src/compiler.c

@@ -1617,7 +1617,14 @@ static void emit_assign_for_tos(Compiler *c, const char *name, uint32_t name_has
         }
         if (!picked) {
             int idx = add_string_constant(c, name);
-            if (local_only) {
+            /* #589: at an IMPORTED module's own top level, a bare write
+             * must never walk past mod_env to rebind whatever the
+             * importer's scope already holds under the same name —
+             * mirrors #373's function-body rule one level up. load_file
+             * doesn't set g_compile_import_toplevel, so its top-level
+             * writes keep walking the chain into the caller's scope,
+             * per its documented "current scope" contract. */
+            if (local_only || g_compile_import_toplevel) {
                 set_op = OP_SET_NAME_LOCAL; set_arg = (uint16_t)idx;
             } else {
                 set_op = OP_SET_NAME; set_arg = (uint16_t)idx;

src/eigenscript.h

@@ -643,6 +643,14 @@ struct EigsThread {
      * global that happened to exist at compile time — the write
      * compiles as a fresh local instead. Reads stay dynamic. */
     int                  compile_module_boundary;
+    /* #589: nonzero ONLY while compiling an `import`ed module's own
+     * top-level statements (never for load_file, whose documented
+     * contract is "runs directly in the current scope"). A bare
+     * `name is expr` at that top level must resolve/create in the
+     * module's own fresh env (mod_env) and never walk through to
+     * whatever the importer's scope happens to already bind — the
+     * top-level counterpart of #373's function-body boundary. */
+    int                  compile_import_toplevel;
     /* Per-import resolution base (Phase 0b). Empty = fall back to
      * state->script_dir. OP_IMPORT saves/restores around module body. */
     char                 import_resolve_dir[4096];
@@ -731,6 +739,7 @@ extern __thread EigsThread *eigs_current;
 #define g_exe_dir             (eigs_current->state->exe_dir)
 #define g_load_env            (eigs_current->load_env)
 #define g_compile_module_boundary (eigs_current->compile_module_boundary)
+#define g_compile_import_toplevel (eigs_current->compile_import_toplevel)
 #define g_import_resolve_dir  (eigs_current->import_resolve_dir)
 #define g_vm_multithreaded    (eigs_current->state->multithreaded)
 #define g_gc_envs             (eigs_current->state->gc_envs)

src/vm.c

@@ -4941,8 +4941,17 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) {
 
         int saved_boundary = g_compile_module_boundary;
         g_compile_module_boundary = 1;                   /* #373 */
+        /* #589: this module's OWN top-level statements compile here too
+         * (mod_env's parent is g_global_env — the importer's scope).
+         * Mark it so a bare top-level `name is expr` binds/updates only
+         * within mod_env and never walks through to the importer's
+         * global — load_file does NOT set this (its documented contract
+         * keeps top-level writes in the caller's current scope). */
+        int saved_import_toplevel = g_compile_import_toplevel;
+        g_compile_import_toplevel = 1;
         EigsChunk *mod_chunk = compile_ast(ast, mod_env, source);
         g_compile_module_boundary = saved_boundary;
+        g_compile_import_toplevel = saved_import_toplevel;
         if (g_parse_errors > 0) {
             g_parse_errors = saved_errors;
             chunk_free(mod_chunk);

tests/run_all_tests.sh

@@ -1709,6 +1709,7 @@ echo ""
 echo "[43a2] Builtin Argument Errors (26 checks)"
 check_eigs_suite "builtin argument errors" test_builtin_errors.eigs "All builtin_errors tests passed" 30
 check_eigs_suite "module-boundary write insulation (#373)" test_module_scope.eigs "All module-scope tests passed" 9
+check_eigs_suite "import top-level scope insulation vs load_file current-scope contract (#589)" test_import_toplevel_scope.eigs "All import top-level scope tests passed" 11
 echo ""
 
 # [43a3] EigenStore header-validation / corruption error paths (ext_store.c)

tests/test_import_toplevel_scope.eigs

@@ -0,0 +1,48 @@
+# #589: import's own top-level bare assignment must bind in the imported
+# module's own scope, never walking through to a same-named binding that
+# happens to already exist in the importer's scope — the top-level
+# counterpart of #373's function-write boundary (test_module_scope.eigs).
+# Before the fix, a module's `counter is 0` at its own top level rebound the
+# IMPORTER's `counter` instead of creating module state, and every later use
+# of `counter` inside the module's functions then resolved to the importer's
+# variable (the exact cross-wire from the issue's minimal repro).
+#
+# load_file keeps its OLDER, documented "runs directly in the current scope"
+# contract (docs/SPEC.md) completely unchanged — this file proves both
+# directions explicitly, since a regression in either one is the failure
+# mode #589 warns about.
+
+# --- Case 1 (import): module top-level state is insulated from a
+# same-named importer variable, both at import time and across repeated
+# calls into the module's own functions. ---
+write_text of ["tmp_589_cmod.eigs", "counter is 0\ndefine bump() as:\n    counter is counter + 1\n    return counter\n"]
+
+counter is [9, 9, 9]
+import tmp_589_cmod
+assert of [(type of counter) == "list", "I1 importer's counter untouched by module top-level init"]
+assert of [counter == [9, 9, 9], "I2 importer's counter value unchanged by import"]
+assert of [(tmp_589_cmod.bump of null) == 1, "I3 module state starts at 1 in its own scope"]
+assert of [(tmp_589_cmod.bump of null) == 2, "I4 module state accumulates in its own scope"]
+assert of [(tmp_589_cmod.bump of null) == 3, "I5 module state keeps accumulating, still module-local"]
+assert of [(type of counter) == "list", "I6 importer's counter still untouched after repeated module calls"]
+assert of [counter == [9, 9, 9], "I7 importer's counter value still unchanged"]
+
+remove_file of "tmp_589_cmod.eigs"
+
+# --- Case 2 (load_file): documented contract is UNCHANGED — a load_file'd
+# file's top-level statements still execute directly in the CURRENT
+# (caller's) scope, so a same-named top-level assignment DOES bind through
+# and DOES clobber the caller's existing binding. This is load_file's older,
+# intentional semantics, not the #589 bug, and the fix must not touch it. ---
+write_text of ["tmp_589_lfmod.eigs", "counterLF is 0\ndefine bumpLF() as:\n    counterLF is counterLF + 1\n    return counterLF\n"]
+
+counterLF is [9, 9, 9]
+load_file of "tmp_589_lfmod.eigs"
+assert of [(type of counterLF) == "num", "L1 load_file's top-level init binds through to the current scope"]
+assert of [counterLF == 0, "L2 current-scope counterLF now holds the loaded file's value"]
+assert of [(bumpLF of null) == 1, "L3 bumpLF reads/writes the (now shared) current-scope binding"]
+assert of [counterLF == 1, "L4 the write is visible in the current scope too — same binding"]
+
+remove_file of "tmp_589_lfmod.eigs"
+
+print of "All import top-level scope tests passed"