fix(loader): preserve canonical identity for source-resolved modules - #110
Merged
Brian Krabach (bkrabach) merged 5 commits intoSep 21, 2026
Merged
Conversation
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Collaborator
Author
|
This PR's head The carrier retains the complete change and passed the combined-state native, consumer, and default Docker smoke gates after CLI companion microsoft/amplifier-app-cli#358 landed. See the combined validation and source receipt and the migration notes in #109. No package was published by this review lane. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Use the same canonical Python module object for source-path validation and runtime mounting. Source resolution now remains authoritative instead of validating one package and then mounting a conflicting installed entry point.
sys.modulesentries. Audit cached children even when the matching root is already loaded.sys.pathchanges by value, including when package initialization reconstructs equivalent path strings; retain import-lock regression coverage.scripts/bump_version.py, with the two local Cargo lock entries synchronized. No external dependency upgrades, new kernel events, Rust algorithms, or public protocol changes.uv.lockroot package with 1.6.2 in a separate lockfile-only commit. All 20 third-party package records, pins, hashes, markers, and URLs are unchanged.Why this is draft
The code and release candidate are available for core-owner review, but this is not a merge/release-ready claim. Required release gates still need resolution before merge. No downstream changes, tag, PyPI publish, or local user-install update is included.
Evidence from this candidate
Independent Python code review; the two import-isolation edge cases found during review have regression tests.
cargo test -p amplifier-core --locked: 505 tests passed across unit, integration, and doctest groups.cargo fmt -p amplifier-core -p amplifier-core-py --check: passed.cargo clippy -p amplifier-core -p amplifier-core-py --locked -- -D warnings: passed.Fresh
maturin build --release --locked: builtamplifier_core-1.6.2-cp311-abi3-manylinux_2_34_aarch64.whlwith the default binding features. SHA-256:f5bbfb07e77133526fbbd646876b41783294f1f988433e7c116817de61c8a66b.Wheel installed into a new isolated Python 3.12 virtual environment. Core, loader, and validator imports resolve to the installed wheel, not the source checkout.
Downstream GitHub
mainchecks: notool.uv.sources.amplifier-coreoverride inamplifier,amplifier-app-cli, oramplifier-foundation. No downstream files changed.Full fresh-wheel Python/binding suite: 1061 passed, 1 failed, 1 skipped, 6 deselected. Failure:
tests/test_hooks_request_id.py::test_concurrent_calls_get_distinct_ids_and_pair_correctlyexpected the agent request before the summarizer request, but observed the reverse. This test and its hook/correlation implementation are outside the patch; the failure remains unresolved and is not waived. Two pytest fixture deprecation warnings also occurred.A bounded diagnostic used five fixed independent runs per build: the freshly built Git-base wheel (
6d4cd217) passed 5/5; this candidate passed 4/5 and failed 1/5. Hook/correlation Python files and the test are byte-identical between the two sources. This does not establish a pre-existing failure or clear the candidate's failing full-suite result. The initial comparison against published PyPI 1.6.1 could not collect this test because that wheel lacksamplifier_core.correlation; the diagnostic therefore used a wheel built from the exact PR base instead.Mandatory
scripts/e2e-smoke-test.sh: not run. It invokes a real Anthropic-backed CLI session; no live-provider credential was used or real-provider release result claimed during this PR preparation.Original code revision
98429cdpassed all five GitHub CI jobs (Rust, Node, Python 3.11–3.13) and CLA: https://github.com/microsoft/amplifier-core/actions/runs/34697441139Latest lockfile-follow-up CI and core-owner approval: pending.
The 12 canonical-module regression cases passed in the focused source test; the same file also passed as part of the fresh-wheel run above. Earlier non-wheel integration evidence is historical only and is not substituted for fresh release evidence.
Lockfile-only follow-up verification
uv lock --check --offlinefailed before the follow-up because the project version was stale. Regenerating withuv lock --offlinechanged only the root package version to1.6.2;uv lock --check --offlinenow passes. The committed lockfile previously recorded1.5.1; the intervening uncommitted1.6.1edit has been superseded by the correct release version.Rust tests, formatting, and Clippy passed again. All 52 Python source files in the existing candidate wheel match the current source tree. One full Python/binding rerun against that wheel passed 1062 tests, with 1 skipped and 6 deselected (the same two fixture deprecation warnings). This successful rerun does not diagnose or waive the earlier intermittent hook-ordering failure above; that gate remains open.
The lockfile change is isolated so it can be reviewed, dropped, or cherry-picked separately. If cherry-picked onto a different release, its root version must match that target's manifest.
Behavioral trade-off / review focus
For a resolver-selected source, reusing an ambient installed entry point with the same module ID is no longer permitted. A conflicting cached source fails explicitly rather than silently mounting or replacing it. Direct entry-point discovery remains supported when no source resolver applies. Please review that conflict policy and the Python import-lock/path-restoration behavior.
This is Python-specific import machinery in the existing Python module loader, not a new cross-language kernel feature. No Rust/Python event or protocol symmetry additions are required.
Before merge
The stale
uv.lockedit is now replaced by the verified release-aligned lockfile update described above. Test-generated protobuf files remain excluded; no proto definition changed.