Skip to content

import: an installed stdlib is the stdlib, not a project file (#904) - #906

Merged
InauguralPhysicist merged 4 commits into
mainfrom
claude/observer-fix-import-collision-u1d3b7
Aug 6, 2026
Merged

import: an installed stdlib is the stdlib, not a project file (#904)#906
InauguralPhysicist merged 4 commits into
mainfrom
claude/observer-fix-import-collision-u1d3b7

Conversation

@InauguralPhysicist

@InauguralPhysicist InauguralPhysicist commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes #904. Found on a second machine with a different configuration — not by CI.

import resolves project-first (#821): <name>.eigs before lib/<name>.eigs, and a name matching both warns. But the resolver chain's tail steps are the install roots<exe>/../lib/eigenscript/ and ~/.local/lib/eigenscript/, what make install and install.sh write — and they answer the bare <name>.eigs request just as readily as lib/<name>.eigs. So on any machine that had installed, the installed stdlib came back as the project hit:

$ src/eigenscript /tmp/imp.eigs          # just `import json`
Warning: import 'json' matches both a project file and a stdlib module — using
'~/.local/lib/eigenscript/json.eigs', shadowing '<repo>/lib/json.eigs'

The stdlib reported as shadowing itself, on every stdlib import.

Two defects, not one

The warning was the visible half. Behind it, project-first then made the installed copy win — over the stdlib shipped next to the binary actually running, and over a bundle's own extracted lib/. A bundle is the one artifact of this project designed to be copied to a machine you don't control, and it was quietly running the host's stdlib instead of the one it carries.

Bundle replay took the visible damage: the replayed run was byte-identical to the recorded one — same tape, same draw, same stdout — and failed its byte-identity check on the one prepended warning line.

Fix

resolve_eigenscript_file_from_ex() reports which half of the chain answered; an install-root hit is the stdlib arm and never the project arm. Genuine project shadowing warns exactly as before.

  • src/builtins_host.c — the _ex resolver, plus the freestanding no-op; the old signature stays as a wrapper, so load_file and lint are untouched
  • src/vm.cOP_IMPORT demotes an install-root "project" hit to the stdlib arm
  • docs/SPEC.md — the project arm means a file you wrote; an installed stdlib is the stdlib arm either way

Regression gates

All three fail on the pre-fix binary (verified by rebuilding HEAD~1's src/) and pass on this one:

  • tests/run_all_tests.sh [36] — no warning, and the right file resolves: the planted install copy has no abs, so a wrong pick fails outright rather than merely warning. A real project shadow still warns exactly once.
  • tests/test_bundle.sh Claude/general session 014 p bt1vzv5w j1t pvd8 ud5fo #11 — an installed stdlib does not shadow the bundle's own lib/, and replay stays byte-identical with one present.
  • .github/workflows/ci.yml — see below.

The two suite gates simulate the install root through HOME rather than requiring an install, so every leg covers it.

The CI leg had this configuration all along

Second commit. install-smoke runs ./install.sh, which builds src/eigenscript in the tree and copies the stdlib to ~/.local/lib/eigenscript — precisely the two-stdlib arrangement that forked resolution, and precisely what a contributor has after following the README. The job never looked: test -x on two binaries and --version, which imports nothing. So the runner sat in the broken state and reported green.

Four lines now import a stdlib module from both the installed binary and the tree binary and require it to resolve with no warning. The tree half fails on the pre-fix binary. (The installed half cannot — with one stdlib both arms resolve to the same file — so it asserts the installed layout resolves at all, which nothing else did either.)

#905 tracks what is still unexercised there: the suite itself, eigenlsp/eigsdap beyond existing, and --api's stdlib index, which walks the same two-root candidate list.

Verification

configuration before after
release, ~/.local/lib/eigenscript present 3806/3810, 4 failed 3815/3815
release, moved aside 3810/3810 3815/3815
asan+ubsan, detect_leaks=1 3819/3819, leak tally 0
make freestanding-check stage 1 + stage 2 OK

Checklist

  • make test passes locally — release in both configurations, and asan+ubsan with detect_leaks=1
  • New builtins have signature comments and docs in docs/BUILTINS.md — n/a, no new builtins
  • New library functions follow conventions in docs/STDLIB.md — n/a, no new library functions
  • New examples have a comment header explaining what they demonstrate — n/a, no new examples
  • CHANGELOG.md updated (if user-facing change)

🤖 Generated with Claude Code

https://claude.ai/code/session_01StzmGd7qXYaYxbpie34Da3

`import` resolves project-first (#821): `<name>.eigs` before
`lib/<name>.eigs`, and a name matching both warns. But the resolver
chain's tail steps are the install roots — `<exe>/../lib/eigenscript/`
and `~/.local/lib/eigenscript/`, what `make install` writes — and they
answer the bare `<name>.eigs` request as readily as `lib/<name>.eigs`.
So on any machine that had run `make install`, the installed stdlib came
back as the *project* hit, and every stdlib import produced two defects:

  Warning: import 'json' matches both a project file and a stdlib
  module — using '~/.local/lib/eigenscript/json.eigs', shadowing
  '<repo>/lib/json.eigs'

the diagnostic reporting the stdlib as shadowing itself, and behind it a
real resolution bug: the installed copy WON, over the stdlib shipped
next to the binary actually running and over a bundle's own extracted
lib/. A bundle — the one artifact here designed to be copied to a
machine you don't control — quietly ran the host's stdlib instead of the
one it carries.

Bundle replay took the visible damage: byte-identical to the recorded
run, same tape, same draw, same stdout, failing its byte-identity check
on the one prepended warning line.

resolve_eigenscript_file_from_ex() now reports which half of the chain
answered; an install-root hit is the stdlib arm and never the project
arm. Genuine project shadowing warns exactly as before.

Found on a second machine, not by CI: every leg runs the suite from the
build tree and none runs `make install`, so the suite was green here and
4-red for anyone following the README's install path (#905 tracks the
missing install leg). Both new gates simulate the install root through
HOME rather than requiring one, so CI covers a configuration it cannot
itself create — they fail on the pre-fix binary and pass on this one.

release 3815/3815 with and without an installed stdlib present (was
3806/3810 with); asan+ubsan detect_leaks=1 3819/3819, leak tally 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StzmGd7qXYaYxbpie34Da3
Copilot AI lite review requested due to automatic review settings August 6, 2026 04:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a resolution-order bug where an installed stdlib (<prefix>/lib/eigenscript/ and ~/.local/lib/eigenscript/) could be misclassified as a “project file” during import’s project-first probe, causing spurious collision warnings and (worse) allowing the installed copy to override the stdlib shipped with the running binary and a bundle’s extracted lib/.

Changes:

  • Add an extended resolver API that reports whether a hit came from install-root stdlib locations, keeping the original resolver as a wrapper for existing callers.
  • Update OP_IMPORT to treat install-root hits as stdlib hits (preventing false collisions and preventing installed stdlib from winning over the binary/bundle stdlib).
  • Add regression gates covering the “installed stdlib present via HOME” scenario for both the main suite and bundle replay, and document the clarified semantics.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_bundle.sh Adds a bundle regression ensuring installed stdlib cannot shadow the bundle’s extracted lib/, preserving replay byte-identity.
tests/run_all_tests.sh Adds suite regression that simulates an installed stdlib via HOME and asserts correct resolution + warning behavior.
src/vm.c Switches import resolution to resolve_eigenscript_file_from_ex and demotes install-root hits from the “project” arm.
src/eigenscript.h Introduces origin constants and declares resolve_eigenscript_file_from_ex.
src/builtins_host.c Implements resolve_eigenscript_file_from_ex (host + freestanding stub) and preserves old API as a wrapper.
docs/SPEC.md Clarifies that installed stdlib roots are always treated as stdlib (never “project”) for import semantics.
CHANGELOG.md Records the user-visible fix and its impact (warnings + resolution correctness + bundle behavior).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`install-smoke` is the only leg that builds in the tree AND installs the
stdlib to `~/.local/lib/eigenscript` — install.sh writes both. That is
exactly the arrangement a contributor has after following the README,
and exactly the one that forked import resolution: two stdlibs, so the
bare `<name>.eigs` probe answered from the install root while
`lib/<name>.eigs` answered from the tree.

The leg never looked. `--version` imports nothing, so the job was green
while every stdlib import on that same runner would have warned that the
stdlib was shadowing itself.

Four lines: import a stdlib module, from both the installed binary and
the tree binary, and require the module to resolve with no warning.
Verified against the pre-fix binary — the tree half fails there. (The
installed half cannot: with one stdlib both arms resolve to the same
file, so it asserts the installed layout resolves at all, which nothing
else did either.)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StzmGd7qXYaYxbpie34Da3

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

tests/run_all_tests.sh:1480

  • This comment says CI has never run make install, but the workflow has an install-smoke leg that runs install.sh and creates the installed stdlib layout; the issue was that CI previously didn’t assert any stdlib imports in that configuration. Reword to avoid an inaccurate root cause description.
# won over the stdlib shipped with the binary being run. CI has never run
# `make install`, which is exactly why this stayed invisible here and was
# found on a second machine; HOME is the lever that simulates the install
# without one. Asserted: no warning, the RIGHT file resolves (the planted

tests/test_bundle.sh:194

  • This comment claims CI never runs make install, but CI does create an installed layout via install.sh in the install-smoke job. Since this test simulates the install root via HOME, the comment can just state that directly (and avoid an inaccurate CI claim).
# same stdout, one extra line. CI never runs `make install`, so the
# install root is simulated through HOME.

CHANGELOG.md:101

  • The changelog entry attributes discovery to CI never running make install, but CI’s install-smoke job runs install.sh and creates the installed layout; the gap was that CI didn’t exercise/validate imports under that layout. Reword to keep the changelog accurate.
  Found on a second machine, not by CI: every CI leg runs the suite from
  the build tree and none runs `make install`, so the suite was green
  there and red for anyone who followed the README's install path first.
  Both new gates simulate the install through `HOME` rather than
  requiring one, so CI covers the configuration it cannot itself create.

The new #904 gate ran the installed binary fine and then hit exit 127 on
`src/eigenscript`. install.sh's second pass is `./build.sh lsp`, and
build.sh opens with `rm -f eigenscript` inside src/ (the #740 hard-link
dance) — so the tree binary is gone by the time install.sh returns,
whether or not anyone wanted it.

Rebuild it, and assert it exists before using it. The two binaries
differ only in where they SIT, which is exactly what is under test:
`<exe>/../lib/` is a resolution step, so the tree binary sees two
stdlibs (repo + install root) and the installed binary sees one.

Verified by running install.sh locally and reproducing the 127, then the
repaired step: green on this branch, and the tree half still fails on
the pre-fix source.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StzmGd7qXYaYxbpie34Da3
Copilot AI review requested due to automatic review settings August 6, 2026 05:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

CHANGELOG.md:101

  • The CHANGELOG entry claims CI “cannot itself create” the installed-stdlib configuration and implies the failure was invisible because CI never installed. However this PR also updates the install-smoke job (which runs install.sh and creates ~/.local/lib/eigenscript) to assert the behavior. It’d be more accurate to describe the prior blind spot as missing assertions/suite coverage for the installed layout rather than CI being unable to create it.
  Found on a second machine, not by CI: every CI leg runs the suite from
  the build tree and none runs `make install`, so the suite was green
  there and red for anyone who followed the README's install path first.
  Both new gates simulate the install through `HOME` rather than
  requiring one, so CI covers the configuration it cannot itself create.

Three places said CI "never runs `make install`". That is wrong, and the
truth is more interesting: install-smoke runs install.sh, which writes
the tree binary AND ~/.local/lib/eigenscript. The configuration was
there. Nothing asserted against it — `--version` imports nothing — so
the runner sat in the broken state and reported green.

"CI cannot create this configuration" also understated what the HOME
simulation buys. It is not a substitute for an install CI lacks; it puts
the install root in front of EVERY leg, including the bundle tests,
which run on a different runner than the one that installs.

Caught by the Copilot reviewer, which flagged the same claim in all
three files. Comment and CHANGELOG text only; no behavior change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StzmGd7qXYaYxbpie34Da3
Copilot AI review requested due to automatic review settings August 6, 2026 06:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@InauguralPhysicist
InauguralPhysicist merged commit 6487916 into main Aug 6, 2026
19 checks passed
@InauguralPhysicist
InauguralPhysicist deleted the claude/observer-fix-import-collision-u1d3b7 branch August 6, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An installed stdlib shadows itself: every import warns, and the installed copy wins over the binary's own stdlib (and over a bundle's extracted lib/)

3 participants