diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index f77dccb7..e479c726 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -11,17 +11,37 @@ name: Dependabot auto-merge # triggers on `main` and folds the security bump into the SAME staging # branch — nothing is ever left sitting on `main` waiting for a human. # -# Merge strategy: the incoming branch ALWAYS clobbers what is already staged -# (`git merge -X theirs`). Successive bumps of the same lock-file never conflict- -# stall: the latest bump wins, every time. Nothing reaches `main` this way — the -# full build/test (ci.yml) + CodeQL (codeql.yml) gate the single -# `dependabot-upgrades -> main` consolidation PR, which is where review and the -# expensive matrix actually run. ci.yml/codeql.yml deliberately SKIP Dependabot -# PRs (they would only burn the matrix on a bump we immediately sweep away). +# Sweep strategy: PATH-RESTRICTED, not a branch merge. Only the files the PR +# itself changed (diff against its merge-base with the PR base) are copied from +# the bump branch onto the staging tip. Two reasons: +# +# * The GITHUB_TOKEN can NEVER push changes to `.github/workflows/*` — the +# `workflows` scope cannot be granted to it, not even via `permissions:`. +# A full `git merge` of a security bump (based on current `main`) into a +# staging branch that has drifted behind `main` drags main's newer commits +# along — including workflow-file edits — and the push is rejected +# (`refusing to allow a GitHub App to ... update workflow`). Copying only +# the bump's own files makes the sweep immune to staging-branch drift. +# * Clobber semantics survive: the latest bump overwrites the same manifest/ +# lock files, so successive bumps never conflict-stall — no merge machinery +# left to conflict at all. +# +# If the bump ITSELF touches `.github/workflows/*` (a github-actions SECURITY +# update — version updates of actions target the staging branch and merge on +# GitHub's side, not here), the token cannot stage it by construction. The +# sweep then leaves the PR open with a comment instead of failing: an actions +# security bump warrants human eyes anyway. +# +# Nothing reaches `main` this way — the full build/test (ci.yml) + CodeQL +# (codeql.yml) gate the single `dependabot-upgrades -> main` consolidation PR, +# which is where review and the expensive matrix actually run. ci.yml/codeql.yml +# deliberately SKIP Dependabot PRs (they would only burn the matrix on a bump we +# immediately sweep away). # # Lives at the repo root so it is present on `dependabot-upgrades` (cut from # main): for `pull_request` the workflow is read from the PR's base branch, so -# BOTH `main` and the staging branch must carry this file. +# BOTH `main` and the staging branch must carry this file. Actions cannot push +# workflow files, so updating the staging branch's copy is always a human push. # # ⚠️ SECURITY INVARIANT — the trigger MUST stay `pull_request`, NEVER # `pull_request_target`. Under `pull_request` a fork PR runs with a READ-ONLY @@ -42,7 +62,7 @@ permissions: jobs: sweep: - name: Clobber-merge into dependabot-upgrades + name: Sweep bump files into dependabot-upgrades # Two independent gates, both required: the (unforgeable) Dependabot actor # AND a `dependabot/*` source branch. actor-AND-source keeps the trust # assumption explicit and resilient if the trigger set is ever widened. @@ -58,34 +78,65 @@ jobs: ref: dependabot-upgrades fetch-depth: 0 - - name: Clobber-merge the bump and retire the PR + - name: Sweep the bump's files and retire the PR env: PR_URL: ${{ github.event.pull_request.html_url }} PR_HEAD: ${{ github.event.pull_request.head.ref }} + PR_BASE: ${{ github.event.pull_request.base.ref }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" # Defense-in-depth: disable git hooks for every git op in this job, so - # merging attacker-influenced tree content can never execute a hook by + # staging attacker-influenced tree content can never execute a hook by # construction. No hook is reachable today (hooks live in .git, not the # tree), but this holds regardless of any future change. git config --global core.hooksPath /dev/null - # Pull the bump branch into a stable local ref we can re-merge. - git fetch origin "+refs/heads/${PR_HEAD}:refs/remotes/origin/${PR_HEAD}" - # Re-merge onto the LIVE staging tip and retry: concurrent Dependabot - # PRs race to push here, so each run rebases on whatever already landed - # and the incoming branch always wins conflicts (-X theirs). + # Pull the bump branch and its base into stable local refs. + git fetch origin \ + "+refs/heads/${PR_HEAD}:refs/remotes/origin/${PR_HEAD}" \ + "+refs/heads/${PR_BASE}:refs/remotes/origin/${PR_BASE}" + # The PR's OWN changes: diff against the merge-base with its base + # branch, so nothing the base has accumulated rides along. + merge_base="$(git merge-base "origin/${PR_BASE}" "origin/${PR_HEAD}")" + changed_list="${RUNNER_TEMP}/dependabot-sweep-paths.z" + git diff -z --name-only "${merge_base}" "origin/${PR_HEAD}" \ + > "${changed_list}" + # The token cannot push workflow files at all; hand those to a human. + while IFS= read -r -d '' path; do + case "${path}" in + .github/workflows/*) + echo "::warning::bump edits ${path}; GITHUB_TOKEN cannot push workflow files — leaving the PR open" + gh pr comment "${PR_URL}" --body \ + "Not swept: this bump edits \`${path}\`, which the Actions token cannot push (no \`workflows\` scope). Please review and merge manually." + exit 0 + ;; + esac + done < "${changed_list}" + # Re-apply onto the LIVE staging tip and retry: concurrent Dependabot + # PRs race to push here, so each run restarts from whatever already + # landed. Copying (not merging) the bump's paths keeps clobber + # semantics: the latest bump of a file wins, every time. for attempt in 1 2 3 4 5; do git fetch origin "+refs/heads/dependabot-upgrades:refs/remotes/origin/dependabot-upgrades" git reset --hard "origin/dependabot-upgrades" - git merge -X theirs --no-edit "origin/${PR_HEAD}" \ - -m "build(deps): clobber-merge ${PR_HEAD} into dependabot-upgrades" + while IFS= read -r -d '' path; do + if git cat-file -e "origin/${PR_HEAD}:${path}" 2>/dev/null; then + git checkout "origin/${PR_HEAD}" -- "${path}" + else + git rm -q --ignore-unmatch -- "${path}" + fi + done < "${changed_list}" + if git diff --cached --quiet; then + echo "bump already staged on dependabot-upgrades; nothing to commit" + break + fi + git commit -q -m "build(deps): sweep ${PR_HEAD} into dependabot-upgrades" if git push origin "HEAD:dependabot-upgrades"; then break fi - if [ "$attempt" = "5" ]; then + if [ "${attempt}" = "5" ]; then echo "::error::could not push to dependabot-upgrades after 5 attempts" exit 1 fi @@ -94,6 +145,6 @@ jobs: # Retire the PR + its branch: the bump is already staged, so the PR # (whether it targeted main or the staging branch) has served its # purpose. `|| true` — GitHub may have auto-closed it on the push. - gh pr close "$PR_URL" --delete-branch \ - --comment "Swept into \`dependabot-upgrades\` (latest bump clobbers previous)." \ - || git push origin --delete "$PR_HEAD" || true + gh pr close "${PR_URL}" --delete-branch \ + --comment "Swept into \`dependabot-upgrades\` (latest bump of a file clobbers the previous one)." \ + || git push origin --delete "${PR_HEAD}" || true diff --git a/crates/basilisk-checker/src/exports.rs b/crates/basilisk-checker/src/exports.rs index 74b7c54a..999fb820 100644 --- a/crates/basilisk-checker/src/exports.rs +++ b/crates/basilisk-checker/src/exports.rs @@ -387,34 +387,49 @@ pub fn load_snapshot_stub_module( source_text, request, target, - |module_name| { - let (logical_uri, body) = match target { - Some(target) => snapshot.read_stub_for_target(module_name, target.python_version), - None => snapshot.read_stub(module_name), - }?; - match target { - Some(target) => basilisk_stubs::pyi_parser::parse_pyi_source_for_target( - body, - Path::new(&logical_uri), - module_name, - stub_source, - basilisk_stubs::StubTier::Tier1, - target, - ) - .ok(), - None => basilisk_stubs::parse_pyi_source( - body, - Path::new(&logical_uri), - module_name, - stub_source, - basilisk_stubs::StubTier::Tier1, - ) - .ok(), - } - }, + |module_name| parse_snapshot_stub(snapshot, target, module_name, stub_source), ) } +/// Read one module's stub body out of an active snapshot and parse it. +/// +/// The single producer of a snapshot-backed [`StubModule`], shared by the +/// cross-module loader above and by the user-stub re-export fallback +/// ([STUBRES-PYI-REEXPORTS], GitHub #312 follow-up): a user stub's star +/// re-export from a stdlib module must see the same target-filtered parse the +/// snapshot's own modules get. +#[must_use] +pub fn parse_snapshot_stub( + snapshot: &basilisk_stubs::typeshed::snapshot::Snapshot, + target: Option<&StubTarget>, + module_name: &str, + stub_source: StubSource, +) -> Option { + let (logical_uri, body) = match target { + Some(target) => snapshot.read_stub_for_target(module_name, target.python_version), + None => snapshot.read_stub(module_name), + }?; + match target { + Some(target) => basilisk_stubs::pyi_parser::parse_pyi_source_for_target( + body, + Path::new(&logical_uri), + module_name, + stub_source, + StubTier::Tier1, + target, + ) + .ok(), + None => basilisk_stubs::parse_pyi_source( + body, + Path::new(&logical_uri), + module_name, + stub_source, + StubTier::Tier1, + ) + .ok(), + } +} + /// Load a `.pyi` external module from immutable VFS text rather than disk. /// Non-stub requests are rejected because `py.typed` modules remain filesystem /// sources at resolution step 5. diff --git a/crates/basilisk-checker/src/imports/apply.rs b/crates/basilisk-checker/src/imports/apply.rs index 7c8bb9f8..bd8e638f 100644 --- a/crates/basilisk-checker/src/imports/apply.rs +++ b/crates/basilisk-checker/src/imports/apply.rs @@ -227,7 +227,10 @@ fn capture_user_stub_api( tier, ) .ok()?; - Some((import.module.clone(), build_stub_api(&stub, stub_path))) + Some(( + import.module.clone(), + build_stub_api(&stub, stub_path, search_paths), + )) } /// Capture the member API of a plain `import X` backed by the **active step-3 @@ -322,7 +325,10 @@ pub fn recapture_user_stub_from_source( tier, ) .ok()?; - Some((import.module.clone(), build_stub_api(&stub, stub_path))) + Some(( + import.module.clone(), + build_stub_api(&stub, stub_path, search_paths), + )) } /// Whether this import binds a user stub — i.e. its member API is re-derived @@ -363,16 +369,36 @@ fn user_stub_path<'a>( /// presence) from a parsed user stub. Names the stub re-exports — redundant /// aliases, `__all__` entries, and star-imported submodules' export sets /// ([STUBRES-PYI-REEXPORTS], GitHub #312) — are members too. +/// +/// Star targets that don't resolve beside the stub fall back to the active +/// step-3 Typeshed source: `MicroPython`'s `uio.pyi` is just `from io import *` +/// with `io` in the custom typeshed's `stdlib/` tree (GitHub #312 follow-up). fn build_stub_api( stub: &basilisk_stubs::StubModule, stub_path: &std::path::Path, + search_paths: &ImportSearchPaths, ) -> ImportedModuleApi { let mut member_names = std::collections::HashSet::new(); member_names.extend(stub.functions.keys().cloned()); member_names.extend(stub.classes.keys().cloned()); member_names.extend(stub.variables.keys().cloned()); member_names.extend(stub.overloads.keys().cloned()); - member_names.extend(basilisk_stubs::reexported_member_names(stub)); + let mut snapshot_fallback = |module_name: &str| { + let active = search_paths.typeshed_snapshot.as_ref()?; + let (snapshot, target) = active.for_importer(Some(stub_path))?; + crate::exports::parse_snapshot_stub( + snapshot, + target, + module_name, + snapshot_stub_source(snapshot), + ) + }; + member_names.extend( + basilisk_stubs::reexports::reexported_member_names_with_fallback( + stub, + &mut snapshot_fallback, + ), + ); ImportedModuleApi { member_names, has_getattr: stub.functions.contains_key("__getattr__"), diff --git a/crates/basilisk-checker/tests/import_apply_tests.rs b/crates/basilisk-checker/tests/import_apply_tests.rs index b823f645..b50b3008 100644 --- a/crates/basilisk-checker/tests/import_apply_tests.rs +++ b/crates/basilisk-checker/tests/import_apply_tests.rs @@ -188,6 +188,203 @@ fn captures_reexports_through_package_init_stub() { let _ = fs::remove_dir_all(&stub_dir); } +/// Regression for GitHub #312 follow-up (comment 5053013115): a user stub may +/// re-export names from a STDLIB stub that resolves through the active step-3 +/// Typeshed source — e.g. `MicroPython`'s `uio.pyi` is just `from io import *`, +/// with `io` living in the custom typeshed's `stdlib/` tree. The star target +/// is outside the user stub's own source root, so following the re-export +/// graph must fall back to the active snapshot; otherwise every re-exported +/// name is a false `imports_module_attribute` ("Module `uio` has no attribute +/// `StringIO`"). +#[test] +fn user_stub_star_reexport_from_stdlib_stub_is_captured() { + let stub_dir = make_tmp_dir("bsk_ir_reexport_stdlib"); + // Mirrors micropython-esp32-stubs' uio.pyi verbatim. + fs::write(stub_dir.join("uio.pyi"), "from io import *\n").unwrap(); + + // `io` exists ONLY in the active custom typeshed, not under stub_dir. + let typeshed = make_custom_typeshed(&[("io.pyi", "class StringIO: ...\nclass BytesIO: ...\n")]); + + let mut paths = make_search_paths(vec![]); + paths.stub_paths = vec![stub_dir.clone()]; + paths.typeshed_snapshot = Some(ActiveTypeshed::new(typeshed, None)); + + let mut resolved = module_with_plain_import("uio"); + resolve_module_imports(&mut resolved, &paths); + + let api = resolved + .imported_modules + .get("uio") + .expect("user stub under stub-paths must be captured"); + for name in ["StringIO", "BytesIO"] { + assert!( + api.member_names.contains(name), + "`{name}` is star-re-exported from the stdlib `io` stub and must be \ + in the captured member API (GitHub #312 follow-up); got {:?}", + api.member_names + ); + } + + let _ = fs::remove_dir_all(&stub_dir); +} + +/// The chained case from the same report: `uasyncio.pyi` is +/// `from asyncio import *`, and the stdlib `asyncio` stub is a PACKAGE whose +/// own API is built out of *relative* re-exports (`from .tasks import *`, +/// `from .tasks import Task as Task`). After crossing into the snapshot, the +/// re-export walk must keep resolving relative star targets *within* the +/// snapshot. +#[test] +fn user_stub_star_reexport_follows_stdlib_package_reexports() { + let stub_dir = make_tmp_dir("bsk_ir_reexport_stdlib_pkg"); + // Mirrors micropython-esp32-stubs' uasyncio.pyi verbatim. + fs::write(stub_dir.join("uasyncio.pyi"), "from asyncio import *\n").unwrap(); + + let typeshed = make_custom_typeshed(&[ + ( + "asyncio/__init__.pyi", + "from .tasks import *\nfrom .tasks import Task as Task\n", + ), + ( + "asyncio/tasks.pyi", + "__all__ = (\"sleep\",)\n\nclass Task: ...\n\nasync def sleep(delay: float) -> None: ...\n", + ), + ]); + + let mut paths = make_search_paths(vec![]); + paths.stub_paths = vec![stub_dir.clone()]; + paths.typeshed_snapshot = Some(ActiveTypeshed::new(typeshed, None)); + + let mut resolved = module_with_plain_import("uasyncio"); + resolve_module_imports(&mut resolved, &paths); + + let api = resolved + .imported_modules + .get("uasyncio") + .expect("user stub under stub-paths must be captured"); + for name in ["sleep", "Task"] { + assert!( + api.member_names.contains(name), + "`{name}` reaches uasyncio via stdlib asyncio/__init__.pyi's own \ + re-exports and must be in the captured member API; got {:?}", + api.member_names + ); + } + + let _ = fs::remove_dir_all(&stub_dir); +} + +/// Real stdlib stubs (`io`, `os`, …) define `__all__`; per runtime +/// `import *` semantics it is authoritative. A cross-boundary star re-export +/// must honour the stdlib stub's `__all__` — include exactly its entries, not +/// every public definition. +#[test] +fn user_stub_star_reexport_honours_stdlib_dunder_all() { + let stub_dir = make_tmp_dir("bsk_ir_reexport_stdlib_all"); + fs::write(stub_dir.join("uio.pyi"), "from io import *\n").unwrap(); + + let typeshed = make_custom_typeshed(&[( + "io.pyi", + "__all__ = (\"StringIO\",)\n\nclass StringIO: ...\nclass BytesIO: ...\n", + )]); + + let mut paths = make_search_paths(vec![]); + paths.stub_paths = vec![stub_dir.clone()]; + paths.typeshed_snapshot = Some(ActiveTypeshed::new(typeshed, None)); + + let mut resolved = module_with_plain_import("uio"); + resolve_module_imports(&mut resolved, &paths); + + let api = resolved + .imported_modules + .get("uio") + .expect("user stub under stub-paths must be captured"); + assert!( + api.member_names.contains("StringIO"), + "`StringIO` is in the stdlib stub's __all__ and must be captured; got {:?}", + api.member_names + ); + assert!( + !api.member_names.contains("BytesIO"), + "`BytesIO` is NOT in the stdlib stub's __all__, so `import *` must not \ + export it; got {:?}", + api.member_names + ); + + let _ = fs::remove_dir_all(&stub_dir); +} + +/// The redundant-alias convention crossing the same boundary: +/// `from io import StringIO as StringIO` in a user stub re-exports the name +/// regardless of where `io` resolves from. +#[test] +fn user_stub_alias_reexport_from_stdlib_stub_is_captured() { + let stub_dir = make_tmp_dir("bsk_ir_reexport_stdlib_alias"); + fs::write( + stub_dir.join("uio.pyi"), + "from io import StringIO as StringIO\n", + ) + .unwrap(); + + let typeshed = make_custom_typeshed(&[("io.pyi", "class StringIO: ...\n")]); + + let mut paths = make_search_paths(vec![]); + paths.stub_paths = vec![stub_dir.clone()]; + paths.typeshed_snapshot = Some(ActiveTypeshed::new(typeshed, None)); + + let mut resolved = module_with_plain_import("uio"); + resolve_module_imports(&mut resolved, &paths); + + let api = resolved + .imported_modules + .get("uio") + .expect("user stub under stub-paths must be captured"); + assert!( + api.member_names.contains("StringIO"), + "a redundant-alias re-export from a stdlib stub must be captured; got {:?}", + api.member_names + ); + + let _ = fs::remove_dir_all(&stub_dir); +} + +/// Guard: the snapshot is a FALLBACK, not an override. `umachine.pyi` star- +/// imports `machine`, which exists as a sibling user stub in the same +/// `stub-paths` dir — that local resolution must keep winning even when the +/// active typeshed also happens to carry a module of the same name. +#[test] +fn user_stub_star_reexport_prefers_sibling_stub_over_snapshot() { + let stub_dir = make_tmp_dir("bsk_ir_reexport_sibling"); + fs::write(stub_dir.join("umachine.pyi"), "from machine import *\n").unwrap(); + fs::write(stub_dir.join("machine.pyi"), "def reset() -> None: ...\n").unwrap(); + + let typeshed = make_custom_typeshed(&[("machine.pyi", "def snapshot_only() -> None: ...\n")]); + + let mut paths = make_search_paths(vec![]); + paths.stub_paths = vec![stub_dir.clone()]; + paths.typeshed_snapshot = Some(ActiveTypeshed::new(typeshed, None)); + + let mut resolved = module_with_plain_import("umachine"); + resolve_module_imports(&mut resolved, &paths); + + let api = resolved + .imported_modules + .get("umachine") + .expect("user stub under stub-paths must be captured"); + assert!( + api.member_names.contains("reset"), + "the sibling `machine.pyi` in the same stub dir must resolve first; got {:?}", + api.member_names + ); + assert!( + !api.member_names.contains("snapshot_only"), + "the snapshot must not shadow a same-root sibling stub; got {:?}", + api.member_names + ); + + let _ = fs::remove_dir_all(&stub_dir); +} + #[test] fn does_not_capture_non_stub_import() { // A plain `.py` source resolution (not a user stub) is not captured. @@ -299,10 +496,20 @@ fn make_custom_typeshed(stdlib_files: &[(&str, &str)]) -> Arc { let identity = SourceIdentity::Custom { digest: "import-apply-custom".to_owned(), }; - let versions = stdlib_files + // One VERSIONS line per TOP-LEVEL module: `os.pyi` → `os`, + // `asyncio/__init__.pyi` and `asyncio/tasks.pyi` both → `asyncio`. + let top_level_modules: std::collections::BTreeSet<&str> = stdlib_files .iter() - .fold(String::new(), |mut versions, (name, _)| { - let module = name.trim_end_matches(".pyi"); + .map(|(name, _)| { + name.split('/') + .next() + .unwrap_or(name) + .trim_end_matches(".pyi") + }) + .collect(); + let versions = top_level_modules + .into_iter() + .fold(String::new(), |mut versions, module| { assert!(writeln!(&mut versions, "{module}: 3.0-").is_ok()); versions }); diff --git a/crates/basilisk-cli/tests/e2e_bundled_typeshed_config.rs b/crates/basilisk-cli/tests/e2e_bundled_typeshed_config.rs index 9d415162..c0720853 100644 --- a/crates/basilisk-cli/tests/e2e_bundled_typeshed_config.rs +++ b/crates/basilisk-cli/tests/e2e_bundled_typeshed_config.rs @@ -111,6 +111,55 @@ fn cli_flags_a_member_the_active_typeshed_stub_does_not_declare() { let _ = std::fs::remove_dir_all(&dir); } +/// Regression for the GitHub #312 follow-up (comment 5053013115), end to end +/// through the real CLI with the reporter's exact configuration: `stub-paths` +/// and `typeshed-path` both point at `typings/`, `typings/uio.pyi` is only +/// `from io import *`, and `io` lives in `typings/stdlib/`. The user stub's +/// star target resolves through the active custom typeshed — never a false +/// "Module `uio` has no attribute `StringIO`". +#[test] +fn cli_accepts_user_stub_reexports_from_the_custom_typeshed_stdlib() { + let dir = unique_dir("user_stub_stdlib_reexport"); + let typings = dir.join("typings"); + let stdlib = typings.join("stdlib"); + std::fs::create_dir_all(&stdlib).expect("create typings/stdlib"); + std::fs::write( + dir.join("pyproject.toml"), + "[project]\nname = \"x\"\nversion = \"0.1.0\"\n\n[tool.basilisk]\nstub-paths = [\"typings\"]\ntypeshed-path = \"typings\"\n", + ) + .expect("write pyproject"); + // Mirrors micropython-esp32-stubs' uio.pyi verbatim. + std::fs::write(typings.join("uio.pyi"), "from io import *\n").expect("write uio stub"); + std::fs::write( + stdlib.join("io.pyi"), + "class StringIO: ...\nclass BytesIO: ...\n", + ) + .expect("write io stub"); + std::fs::write(stdlib.join("VERSIONS"), "io: 3.0-\n").expect("write VERSIONS"); + std::fs::write( + dir.join("app.py"), + "import uio\n\nbuffer_1 = uio.StringIO()\nbuffer_2 = uio.BytesIO()\n", + ) + .expect("write app"); + + let output = check_app(&dir); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!( + !stdout.contains("imports_module_attribute"), + "`StringIO`/`BytesIO` are star-re-exported from the custom typeshed's \ + `io` stub and must not be flagged, stdout: {stdout}, stderr: {stderr}" + ); + assert_eq!( + output.status.code(), + Some(0), + "spec-valid re-exports must let the CLI check pass, stdout: {stdout}, stderr: {stderr}" + ); + + let _ = std::fs::remove_dir_all(&dir); +} + /// The other half of GitHub #330, and the guard against re-introducing #312: /// members the active Typeshed stub DOES declare — including names it /// re-exports rather than defines — must never be flagged. Capturing the diff --git a/crates/basilisk-stubs/src/reexports.rs b/crates/basilisk-stubs/src/reexports.rs index 89eb8925..5430201e 100644 --- a/crates/basilisk-stubs/src/reexports.rs +++ b/crates/basilisk-stubs/src/reexports.rs @@ -23,13 +23,32 @@ use crate::types::{ /// sets of star-imported stubs resolved relative to the stub's path. #[must_use] pub fn reexported_member_names(stub: &StubModule) -> HashSet { + reexported_member_names_with_fallback(stub, &mut |_| None) +} + +/// Like [`reexported_member_names`], but with a `fallback` loader consulted +/// for star targets that do not resolve inside the stub's own source root. +/// +/// A user stub may re-export from a module owned by a DIFFERENT stub source — +/// `MicroPython`'s `uio.pyi` is just `from io import *`, with `io` living in +/// the active typeshed's `stdlib/` tree (GitHub #312 follow-up). The stub's +/// own root always resolves first, so a sibling stub keeps shadowing any +/// same-named fallback module. +#[must_use] +pub fn reexported_member_names_with_fallback( + stub: &StubModule, + fallback: &mut impl FnMut(&str) -> Option, +) -> HashSet { let root = source_root(stub); let source = stub.source; let tier = stub.tier; let target = stub.target.clone(); let mut loader = |module_name: &str| { - let path = resolve_dotted_stub(&root, module_name)?; - parse_filesystem_stub(&path, module_name, source, tier, target.as_ref()) + resolve_dotted_stub(&root, module_name) + .and_then(|path| { + parse_filesystem_stub(&path, module_name, source, tier, target.as_ref()) + }) + .or_else(|| fallback(module_name)) }; reexported_member_names_with_loader(stub, &mut loader) } diff --git a/docs/specs/CHECKER-STUB-RESOLUTION-SPEC.md b/docs/specs/CHECKER-STUB-RESOLUTION-SPEC.md index 126d08a0..7507f1dd 100644 --- a/docs/specs/CHECKER-STUB-RESOLUTION-SPEC.md +++ b/docs/specs/CHECKER-STUB-RESOLUTION-SPEC.md @@ -497,6 +497,16 @@ name from only one branch. This follows the pinned directive that checkers are expected to understand those checks ([`python/typing@6ef9f77`, directives](https://github.com/python/typing/blob/6ef9f7719ecfff09dad8724ef42b621fd994fb5e/docs/spec/directives.rst)). +Star targets resolve **inside the re-exporting stub's own source root first**; +a target absent there falls back to the active step-3 Typeshed source +(GitHub #312 follow-up). A user stub may legitimately re-export from a module +owned by a different stub source — MicroPython's `uio.pyi` is just +`from io import *` with `io` in the custom typeshed's `stdlib/` tree — and the +walk keeps recursing through whichever source resolved each target, so a +stdlib package reached this way still follows its own relative re-exports +within the snapshot. Local-first ordering means a sibling stub always shadows +a same-named fallback module; the snapshot is a fallback, never an override. + --- ## Type Provenance {#STUBRES-PROVENANCE} diff --git a/vscode-extension/.vscode-test.mjs b/vscode-extension/.vscode-test.mjs index 08c353cb..c6d20210 100644 --- a/vscode-extension/.vscode-test.mjs +++ b/vscode-extension/.vscode-test.mjs @@ -85,6 +85,7 @@ export default defineConfig({ reporter: 'list', timeout: 45_000, require: './out/test/suite/index.js', + ...(process.env.BSK_TEST_GREP ? { grep: process.env.BSK_TEST_GREP } : {}), }, }, ...realWorldTests], coverage: { diff --git a/vscode-extension/package-lock.json b/vscode-extension/package-lock.json index 9fb0203d..ec4c7b90 100644 --- a/vscode-extension/package-lock.json +++ b/vscode-extension/package-lock.json @@ -3799,9 +3799,9 @@ } }, "node_modules/linkify-it": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-5.0.1.tgz", - "integrity": "sha512-wVoTjP4Q6R0NW5hiZkVJaFZPWgtXfoGF+6LucL3/FtiNjmcHhYjEr5f1Kqjirc1nBW07J/ZuRFumqr2oqccEWg==", + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-5.0.2.tgz", + "integrity": "sha512-ONTm2jCMAVZjgQa/Fy1kScXsuOoF5NPTsoFBdE1KVIZ2vAh/r9+Bqo+0jINCBYnavTPQZz38QzFTme79ENoN3Q==", "dev": true, "funding": [ { diff --git a/vscode-extension/src/configuration-editor-script-core.ts b/vscode-extension/src/configuration-editor-script-core.ts index f6cb440f..b5bbcfc7 100644 --- a/vscode-extension/src/configuration-editor-script-core.ts +++ b/vscode-extension/src/configuration-editor-script-core.ts @@ -20,6 +20,12 @@ export const CONFIGURATION_EDITOR_SCRIPT_CORE = String.raw` let activeTag; let selectedRuleCode; let lastFocusedRule; + // Whether the live preview's discard has already been reported to the + // host. The dialog's 'close' event arrives on a QUEUED task that a + // throttled (occluded) webview may never run, so every discard + // initiation point posts synchronously and this flag keeps the close + // event's fallback from double-posting. Re-armed on each preview state. + let previewCancelReported = false; // Which navigation view is visible. Rules is the default so the editor // opens on the tag-first rule browser exactly as before. let activeSection = 'rules'; diff --git a/vscode-extension/src/configuration-editor-script-events.ts b/vscode-extension/src/configuration-editor-script-events.ts index cc65c48e..168b1ef4 100644 --- a/vscode-extension/src/configuration-editor-script-events.ts +++ b/vscode-extension/src/configuration-editor-script-events.ts @@ -54,11 +54,23 @@ export const CONFIGURATION_EDITOR_SCRIPT_EVENTS = String.raw` event.preventDefault(); return true; } + // Discarding a preview must reach the host even if the dialog's queued + // 'close' event never runs (an occluded webview throttles that task + // source), so the intent posts HERE, synchronously with the user's + // action; closing the dialog is only the visual half. + function discardPreview() { + if (editorState.phase === 'preview' && !previewCancelReported) { + previewCancelReported = true; + vscode.postMessage({ type: 'cancelPreview' }); + } + const previewDialog = byId('preview-dialog'); + if (previewDialog.open) previewDialog.close(); + } function handleAction(action) { if (action === 'refresh') vscode.postMessage({ type: 'refresh' }); else if (action === 'open-raw') vscode.postMessage({ type: 'openRaw' }); else if (action === 'load-more-occurrences') loadMoreOccurrences(); - else if (action === 'close-preview') byId('preview-dialog').close(); + else if (action === 'close-preview') discardPreview(); else if (action === 'apply-preview' && editorState.phase === 'preview') vscode.postMessage({ type: 'apply' }); else if (action === 'adopt-workspace') vscode.postMessage({ type: 'adopt', scope: 'workspace' }); else if (action === 'fix-safe') vscode.postMessage({ type: 'fixSafe' }); @@ -121,8 +133,14 @@ export const CONFIGURATION_EDITOR_SCRIPT_EVENTS = String.raw` // A dialog dismissed any way at all (button, Escape, backdrop) discards // the change: the host returns to the snapshot and every control // re-renders from it, so nothing on screen can outlive the decision. + // User-initiated paths post through discardPreview() synchronously; this + // listener is the fallback for any other close so a discard can still + // never be lost, guarded against double-posting. byId('preview-dialog').addEventListener('close', () => { - if (editorState.phase === 'preview') vscode.postMessage({ type: 'cancelPreview' }); + if (editorState.phase === 'preview' && !previewCancelReported) { + previewCancelReported = true; + vscode.postMessage({ type: 'cancelPreview' }); + } }); byId('rule-search').addEventListener('input', applyFilter); byId('rule-viewport').addEventListener('scroll', () => window.requestAnimationFrame(renderRuleWindow), { passive: true }); @@ -139,7 +157,7 @@ export const CONFIGURATION_EDITOR_SCRIPT_EVENTS = String.raw` showSection('rules'); byId('rule-search').focus(); } - if (event.key === 'Escape' && byId('preview-dialog').open) byId('preview-dialog').close(); + if (event.key === 'Escape' && byId('preview-dialog').open) discardPreview(); }); showSection(activeSection); vscode.postMessage({ type: 'ready' }); diff --git a/vscode-extension/src/configuration-editor-script-render.ts b/vscode-extension/src/configuration-editor-script-render.ts index 5c7d6a71..ffa7ca36 100644 --- a/vscode-extension/src/configuration-editor-script-render.ts +++ b/vscode-extension/src/configuration-editor-script-render.ts @@ -364,6 +364,9 @@ export const CONFIGURATION_EDITOR_SCRIPT_RENDER = String.raw` status.textContent = editorState.message || editorState.phase; if (snapshot) renderSnapshot(); renderNotice(); + // Each preview state re-arms the discard report: this preview has not + // been cancelled yet, whatever happened to the previous one. + if (editorState.phase === 'preview') previewCancelReported = false; if (preview && editorState.phase === 'preview') renderPreview(); const dialog = byId('preview-dialog'); if (editorState.phase !== 'preview' && dialog.open) dialog.close(); diff --git a/vscode-extension/src/test/suite/configuration-editor-typeshed-dom.test.ts b/vscode-extension/src/test/suite/configuration-editor-typeshed-dom.test.ts index 3136f2b8..7e2177e4 100644 --- a/vscode-extension/src/test/suite/configuration-editor-typeshed-dom.test.ts +++ b/vscode-extension/src/test/suite/configuration-editor-typeshed-dom.test.ts @@ -226,9 +226,11 @@ const dialogDriver = String.raw` const opened = record('dialog-open'); opened.ruleValue = ruleValue(); // ...and dismissing it discards the change: the control must snap back. - // Closing a dialog that is already open is what fires the close event, so - // the wait above is also what makes the discard reach the host at all. - dialog().close(); + // Dismiss through the Cancel button exactly as a user would: the + // discard intent posts synchronously with the click. A programmatic + // dialog.close() would instead lean on the QUEUED 'close' event, which + // an occluded webview's throttled task queue may never deliver. + await click(el('[data-action="close-preview"]')); await waitUntil(() => !dialog().open); await sleep(settleDelay); const cancelled = record('dialog-cancelled'); diff --git a/website/package-lock.json b/website/package-lock.json index f2fb7749..f20ce857 100644 --- a/website/package-lock.json +++ b/website/package-lock.json @@ -1580,9 +1580,9 @@ } }, "node_modules/linkify-it": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-5.0.1.tgz", - "integrity": "sha512-wVoTjP4Q6R0NW5hiZkVJaFZPWgtXfoGF+6LucL3/FtiNjmcHhYjEr5f1Kqjirc1nBW07J/ZuRFumqr2oqccEWg==", + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/linkify-it/-/linkify-it-5.0.2.tgz", + "integrity": "sha512-ONTm2jCMAVZjgQa/Fy1kScXsuOoF5NPTsoFBdE1KVIZ2vAh/r9+Bqo+0jINCBYnavTPQZz38QzFTme79ENoN3Q==", "dev": true, "funding": [ {