Register PEP 723 scripts as exact projects (PEP 723 PR 10/16) - #1744
Register PEP 723 scripts as exact projects (PEP 723 PR 10/16)#1744Stella Huang (StellaHuang95) wants to merge 1 commit into
Conversation
0201157 to
5784941
Compare
|
🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR. |
| if (!project || normalizePath(project.uri.fsPath) !== normalizePath(scope.fsPath)) { | ||
| return undefined; | ||
| } | ||
| const exactSetting = getExactPythonProjectSetting(this.pm, scope); |
There was a problem hiding this comment.
Warning · Non-blocking recommendation
This ignores the managed exact-project setting unconditionally, so once inline routing becomes stale or disabled, manager resolution falls through to the workspace default rather than the stored fallback. Only bypass this setting while inline routing is active, and cover a case where the fallback and workspace-default manager IDs differ.
| return workspaceApis.getWorkspaceFile() | ||
| ? ConfigurationTarget.WorkspaceFolder | ||
| : ConfigurationTarget.Workspace; | ||
| } |
There was a problem hiding this comment.
Warning · Non-blocking recommendation
Selecting a target based on whether it contains any entries can miss a matching user project at another target; for example, an unrelated workspace-folder entry causes a matching workspace entry to be shadowed by a new created entry. Search defined targets for the matching project first, then choose a target only when creating a genuinely new entry; cover this mixed-scope case with a stateful test.
| return scopes.some((uri) => | ||
| getManagedInlineScriptProjectRegistration(getExactPythonProjectSetting(this.pm, uri)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Warning · Non-blocking recommendation
A non-inline selection issued immediately after an inline selection can bypass this queue because the managed marker is installed asynchronously, allowing invocation order to be lost. Track pending inline scopes synchronously or serialize all persisted exact-script selections, and test by pausing registration before issuing the ordinary selection.
|
Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified. Result: Summary: Verification could not run because no trusted sandbox image is configured for this repository, and local execution was not authorized. The three focused unit suites were therefore not run. I identified 12 tests added by the PR covering registration, rollback, fallback routing, cleanup, and multi-root behavior. Runtime confidence remains low until the sandbox configuration is available. Test runs: 4 not run
|
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
5784941 to
d85e383
Compare
Persist extension-managed per-script project entries while preserving ordinary fallback settings and user-owned entries. Coordinate registration, rollback, explicit manager changes, and cache cleanup. Part of microsoft#1602. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d85e383 to
ee5d7f0
Compare
Update sections 6, 7, 9, and 10 to reflect what was actually built and the in-flight PRs (vscode-python-environments microsoft#1744/microsoft#1745, pyrx #9265, vscode-python #26129): - Q6: dedicated InlineScriptAssociationStore + metadata-identity binding (pending/matched) instead of reusing VenvManager persistence; clarify pythonProjects[] is user-visible registration, not routing. - Q7: TTL runs once per session (runTtlEvictionOnce) and protects entries referenced by a live association (PR microsoft#1745). - Q9: Pylance re-routes via the existing didChangeConfiguration signal (_revalidateOpenRegularFiles / revalidateWorkspaceForFile), so PR 18's dedicated notification is optional; fix persistence/event-source bullets. - Q10: correct the '~10 LOC' estimate to the real exactResource + __pythonIsProgramInterpreter + middleware change; add useEnvExtension gating caveat for Run and Debug. Add a status banner noting the doc was revised post-implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Roadmap context
This is PR 10 of 16 in the PEP 723 inline-script roadmap. PRs 7-9 persist, discover, validate, and route per-script environments; this PR gives each configured script an exact project identity that survives restart and can be consumed by per-file integrations.
Why this PR
PR 9 can route a saved script to a validated inline environment, but the project manager still identifies the script through its containing workspace project. That prevents the per-file identity from surviving restart consistently and leaves downstream configuration and environment-change consumers without an exact script scope.
The registration also needs an ownership boundary: clearing inline environments must remove entries created by the extension without deleting user-authored project settings.
What this PR does
pythonProjectsentry before binding an inline environment.created: remove the entry during inline cleanup;adopted: remove only the marker and preserve the user's entry.Registration and cleanup semantics
workspacediscriminatorPerformance and safety
User impact
The feature remains behind
python-envs.inlineScripts.enabled. Existing users and projects without a managed inline-script entry retain their current manager-selection behavior. After setup, a script has a stable exact project scope across reloads; when inline routing is unavailable, its previous project/workspace environment remains the fallback.Tests
npm run compile-testsnpm run compilenpm run lintScope and follow-up
This PR does not add the setup CodeLens, bulk setup command, TTL eviction, or public feature enablement. Those remain in PRs 11, 12, and 14. Per-file language-service and debugger integration are handled by the companion cross-repository PRs above.