Skip to content

Add inline script activation-time discovery (PEP 723 PR 8/16) - #9

Closed
StellaHuang95 wants to merge 4 commits into
mainfrom
pep723-pr8-activation-discovery-v2
Closed

Add inline script activation-time discovery (PEP 723 PR 8/16)#9
StellaHuang95 wants to merge 4 commits into
mainfrom
pep723-pr8-activation-discovery-v2

Conversation

@StellaHuang95

Copy link
Copy Markdown
Owner

Clean-history replacement for #6. Fork-review PR; do not send upstream yet.

Part of microsoft#1602. This is roadmap PR 8 of 16 and builds on merged PR 7.

Summary

  • discover valid cached inline-script environments after activation
  • expose discovered entries through getEnvironments('all')
  • reconcile add/remove changes and emit manager environment events
  • keep activation non-blocking with deferred, bounded bootstrap retries
  • keep explicit refresh() a settled, single-pass operation

Safety and lifecycle

  • validates sidecars, base interpreters, venv launchers, direct-child ownership, and cache locks
  • preserves transient, unavailable, and busy entries rather than deleting them
  • removes stale published entries when their launcher or metadata becomes definitively invalid
  • coalesces overlapping scans and cancels bootstrap retries on explicit refresh or disposal
  • fixes Windows discovery to verify both the cached launcher and its base interpreter

Scope

No automatic routing, project registration, setup UI, cache deletion, TTL eviction, or lifecycle telemetry.

Validation

  • TypeScript test compilation
  • ESLint
  • focused unit suite: 176 passing, 3 expected platform skips
  • three manual review passes; no significant findings remain

@StellaHuang95

Copy link
Copy Markdown
Owner Author

🔒 Automated review in progress — @StellaHuang95 is auto-reviewing this PR.

Comment thread src/test/common/inlineScript/cacheLayout.unit.test.ts
Comment thread src/test/managers/builtin/inlineScript/envManager.unit.test.ts
Comment thread src/managers/builtin/inlineScript/envManager.ts
Comment thread src/managers/builtin/inlineScript/envManager.ts
Comment thread src/managers/builtin/inlineScript/envManager.ts
Comment thread src/managers/builtin/inlineScript/envManager.ts
@StellaHuang95 StellaHuang95 added the review-auto:changes-requested Automated review: posted blocking findings to address. label Aug 17, 2026
## Summary

Adds a package-manager-centric integration baseline that intentionally
precedes and de-risks microsoft#1686, so the package-manager command refactor is
exercised against behavior established on `main`.

- drives one stateful install/list/direct-package/uninstall lifecycle
per active profile
- uses unique disposable projects and manager-owned disposable
environments
- exercises the live registered manager instances through a
runtime-gated integration-test bridge
- guards registry completeness so every registered package-manager ID
has an active fixture or explicit deferral
- covers normal Pip execution and Conda when their runtime prerequisites
are available
- records an uncached baseline instead of assuming a newly created
environment is empty
- restores workspace-scoped configuration from `inspect()` snapshots and
performs guarded failure-safe cleanup
- defers Poetry pending a Poetry-owned project/lockfile lifecycle
- defers uv-backed Pip because changing the machine-scoped selection
reliably within one extension host was not stable on `main`, while
available-version lookup would also introduce `uv tool run pip` network
seeding
- pins the disposable integration-test user profile to normal Pip
execution

## Validation

- `npm run compile`
- `npm run compile-tests`
- `npm run lint`
- `npm run unittest`
- targeted `packageManagement.integration.test.js`: 3 passing, 2
prerequisite skips locally
- Pip skipped because quick create selected Python 3.15.0 alpha, whose
bundled Pip metadata is incomplete
  - Conda skipped because Conda is not installed
- reviewer specialist: clean, no Critical or Important findings

The active Pip and Conda fixtures require package-index/network access
when their runtime prerequisites are present.

Fixes microsoft#1701

---------

Copilot-Session: 6b2fe9b5-38ea-442f-b07a-b6c71134d480
Copilot-Session: 3fd1a810-6840-4ac9-ac33-c8a9fda4bfc4
@StellaHuang95

Copy link
Copy Markdown
Owner Author

The post-snapshot discovery gap is also addressed in ffaa39c. Activation discovery compares stable snapshots and schedules a bounded final follow-up when entries appear during a pass or a build remains transient. Refresh coalescing records the pass mode, so activation joining an explicit single-pass refresh receives its own snapshot-aware follow-up; explicit refresh callers still await only their requested pass. Retry work is coalesced, disposal-aware, and adds no default-off activity.

@StellaHuang95 StellaHuang95 added review-auto:approved Automated review: no blocking findings (approval posted). and removed review-auto:changes-requested Automated review: posted blocking findings to address. labels Aug 18, 2026
mrleemurray and others added 3 commits August 18, 2026 11:07
)

Replace the existing activity bar icon with a new design more aligned
with the wider codicon design language.


![image.png](https://github.com/user-attachments/assets/65b0c66b-0e1a-4610-89d7-94206fafa044)

Co-authored-by: mrleemurray <mrleemurray@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3cb82ae9-7424-40a4-9156-8c54ac6e0895
Use stable cache identities, fail closed on lock probes, and retry snapshot changes safely.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2
@StellaHuang95
StellaHuang95 force-pushed the pep723-pr8-activation-discovery-v2 branch from ffaa39c to faa729f Compare August 18, 2026 21:42
@StellaHuang95

Copy link
Copy Markdown
Owner Author

Upstream review continues in microsoft#1722. Closing this fork-only review PR as superseded.

assert.strictEqual(await getBaseInterpreterStatus(envDir), 'missing');
assert.strictEqual(await verifyBaseInterpreterExists(envDir), false);
assert.ok(
traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('not a regular file')),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Issue · Please address or respond

The warning assertions only check substrings. Capture the complete warning values and assert them exactly, as required by the repository test-assertion rules.

resolveVenvStub.onSecondCall().resolves(environment);
const retryManager = manager as unknown as {
getDiscoveryRetryDelayMs(attempt: number): number | undefined;
};

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Issue · Please address or respond

The private-method cast and retry-delay stub setup is repeated across several tests. Extract a narrow suite-local helper that configures the discovery retry schedule.

return { kind: 'resolved', environment };
}

private replaceDiscoveredEnvironments(next: PythonEnvironment[]): void {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Issue · Please address or respond

next is never mutated. Accept a ReadonlyArray<PythonEnvironment> and make the stored collection readonly-array-valued while retaining the defensive copy returned by getEnvironments.

}

return { kind: 'resolved', environment };
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

Discovery independently repeats metadata, interpreter, ownership, and version validation already represented by the reuse pipeline. Extract a shared low-level physical-integrity result and map it to the distinct discovery and reuse policies.

}
this.scheduleActivationDiscoveryRetry();
})
.catch((error) => {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

The manager now owns refresh coalescing, snapshot follow-ups, retry scheduling, scanning, reconciliation, events, and disposal. Move this state machine into a focused discovery coordinator so InlineScriptEnvManager does not continue accumulating unrelated lifecycle responsibilities.

Comment thread src/api.ts
@@ -872,47 +883,63 @@ export interface GetPackagesOptions {
skipCache?: boolean;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Warning · Non-blocking recommendation

The package-management API, network workflows, integration harness, pip behavior, and logo replacement are unrelated to the inline-script discovery described by this PR. Split those changes into independently reviewable PRs so this correctness change retains a coherent scope.

@StellaHuang95 StellaHuang95 added review-auto:changes-requested Automated review: posted blocking findings to address. and removed review-auto:approved Automated review: no blocking findings (approval posted). labels Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-auto:changes-requested Automated review: posted blocking findings to address.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants