Skip to content

feat(python): auto-activate venv before hook execution#347

Merged
zimeg merged 20 commits intomainfrom
mwbrooks-python-venv-activation
Feb 27, 2026
Merged

feat(python): auto-activate venv before hook execution#347
zimeg merged 20 commits intomainfrom
mwbrooks-python-venv-activation

Conversation

@mwbrooks
Copy link
Member

@mwbrooks mwbrooks commented Feb 21, 2026

Changelog

When running Slack CLI commands in a Bolt Python project, the CLI will attempt to automatically activate the venv. This will allow commands that use the slack-cli-hooks Python package to safely and successfully run even when the terminal system doesn't have the virtual environment activated.

Summary

This pull request gives Python the "create and run" experience that Node and Deno enjoy.

It enables auto-activation of venv, when it exists, so that commands can run safely and successfully even when the terminal session doesn't have a virtual environment activated. When the .venv is missing, the CLI will continue like normal.

Test coverage:

  • I noticed that the test coverage is reported as being low
  • Checking locally, it should be higher than reported
  • I'm hopeful that once mwbrooks-python-venv is merged, this branch will report higher coverage

Merge order:

  • This PR depends on the branch mwbrooks-python-venv
  • First merge mwbrooks-python-venv then merge this PR into main

Preview

2026-02-20-create-run-python.mov

Reviewers

Bolt for Python:

$ lack create my-test-app/
# → Select "AI Agent App"
# → Select "Bolt for Python"

# Validate output:
# → Confirm: "Created virtual environment at .venv"
# → Confirm: "Installed dependencies from pyproject.toml"
# → Confirm: "Installed dependencies from requirements.txt"
# → Confirm: No errors

# Run the app
$ cd my-test-app/
$ lack run
# → Confirm: App starts

# Clean up
$ cd ../
$ rm -rf my-test-app/

Bolt for JavaScript:

$ lack create my-test-app/
# → Select "AI Agent App"
# → Select "Bolt for JavaScript" <---- Not Python

# Validate output:
# → Confirm: No errors

# Run the app
$ cd my-test-app/
$ lack run
# → Confirm: App starts

# Clean up
$ cd ../
$ rm -rf my-test-app/

Requirements

Swap pip install order so pyproject.toml is installed first to set up
the project package, then requirements.txt pins take precedence as the
lockfile.
Set VIRTUAL_ENV, prepend venv bin to PATH, and unset PYTHONHOME on the
CLI process before any hooks run. This lets hook scripts (e.g.
get-hooks, start, deploy) resolve the venv's Python and installed
packages without requiring developers to manually activate the venv.

Activation happens in InitSDKConfig after finding the project root and
before reading/executing hooks. It is a no-op when no .venv exists, so
non-Python projects are unaffected. Failure is non-fatal (debug log).
Clarify error messages in installPyProjectToml to include "updating
pyproject.toml" context, and remove early return on file update errors
so pip install is attempted regardless.
Route createVirtualEnvironment and runPipInstall through HookExecutor
instead of os/exec, matching the npm runtime pattern. This makes the
functions testable with the mock filesystem. Also add missing
AddDefaultMocks call, remove stale test cases, and fix assertion strings.
Return a boolean from ActivateVenvIfPresent to indicate whether
activation occurred, and log a debug message on success.
@mwbrooks mwbrooks added this to the Next Release milestone Feb 21, 2026
@mwbrooks mwbrooks self-assigned this Feb 21, 2026
@mwbrooks mwbrooks added enhancement M-T: A feature request for new functionality changelog Use on updates to be included in the release notes semver:minor Use on pull requests to describe the release version increment area:bolt-python Related to github.com/slackapi/bolt-python labels Feb 21, 2026
@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 41.66667% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.67%. Comparing base (3417188) to head (8554adc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/runtime/python/python.go 50.00% 4 Missing and 4 partials ⚠️
internal/shared/clients.go 0.00% 2 Missing and 2 partials ⚠️
internal/slackdeps/os.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #347   +/-   ##
=======================================
  Coverage   64.66%   64.67%           
=======================================
  Files         213      213           
  Lines       18018    18042   +24     
=======================================
+ Hits        11652    11668   +16     
- Misses       5277     5282    +5     
- Partials     1089     1092    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member Author

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

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

Comments for those kind reviewers! 🙇🏻

Comment on lines 106 to 112
if err := os.Setenv("VIRTUAL_ENV", venvPath); err != nil {
return false, err
}
if err := os.Setenv("PATH", binDir+string(filepath.ListSeparator)+os.Getenv("PATH")); err != nil {
return false, err
}
os.Unsetenv("PYTHONHOME")
Copy link
Member Author

Choose a reason for hiding this comment

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

note: I imagine we should create wrappers for these calls in our slackdeps.Os 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

note: I updated this PR to use slackdeps.Os and added slackdeps.Os.lookupEnv.

Copy link
Member

Choose a reason for hiding this comment

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

@mwbrooks 🌲 So good! I think we'll want to reuse this in other places soon?

Comment on lines 69 to 76
// ActivatePythonVenvIfPresent activates a Python virtual environment if one
// exists in the given project directory. This sets VIRTUAL_ENV, prepends the
// venv bin directory to PATH, and unsets PYTHONHOME on the current process so
// that child processes (hook scripts) inherit the activated venv.
func ActivatePythonVenvIfPresent(fs afero.Fs, projectDir string) (bool, error) {
return python.ActivateVenvIfPresent(fs, projectDir)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

note: While it feels wrong to have Python in the generic runtime this allows us to activate it for any project. We could move this into the Python runtime and think of a more general function that could be used across all runtimes.

Upside of this approach is that JS projects that include a .venv (perhaps for scripts) can be activated automatically.

if activated, err := runtime.ActivatePythonVenvIfPresent(c.Fs, dirPath); err != nil {
c.IO.PrintDebug(ctx, "failed to activate Python virtual environment: %s", err)
} else if activated {
c.IO.PrintDebug(ctx, "Activated Python virtual environment .venv")
Copy link
Member Author

Choose a reason for hiding this comment

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

note: I'd love to display this in the regular output, but it's tough to get a nice format because it can be displayed in many different situations.

Copy link
Member

Choose a reason for hiding this comment

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

⚠️ suggestion(non-blocking): Your approach is solid, but we might consider outputting the errors as a warning while keeping expected activations to the debug logs?

Would love to hear your thoughts on this! IIRC logic similar to what's here exists for the get-hooks activation, but we often miss problems that should be at least reported. Even if the command continues I think!

…bility

Add Unsetenv to the Os interface/implementation/mock and refactor
ActivateVenvIfPresent to accept types.Os instead of calling the os
package directly. This allows the test to use mocks instead of mutating
the real process environment.
@mwbrooks mwbrooks marked this pull request as ready for review February 21, 2026 03:06
@mwbrooks mwbrooks requested a review from a team as a code owner February 21, 2026 03:06
Base automatically changed from mwbrooks-python-venv to main February 27, 2026 02:58
Adopt map-based table-driven test pattern from main while preserving
new ActivateVenvIfPresent and getVenvBinDir functions and their tests.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

@mwbrooks This feels awesome to use! The steps after create are much more quick now and apps feel "lighter" to run somehow... 🐍

I'm merging this after leaving a few comments that weren't blocking but might be nice to follow up on as we encounter edges in approach - thanks for bringing this to the finish 🏁 ✨

Comment on lines +79 to +85
// getVenvBinDir returns the platform-specific bin directory inside a virtual environment
func getVenvBinDir(venvPath string) string {
if runtime.GOOS == "windows" {
return filepath.Join(venvPath, "Scripts")
}
return filepath.Join(venvPath, "bin")
}
Copy link
Member

Choose a reason for hiding this comment

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

👁️‍🗨️ issue(non-blocking): I'm finding the windows option is erroring with perhaps unrelated hook errors in execution explored in slackapi/python-slack-hooks#96

Comment on lines +99 to +101
if !venvExists(fs, venvPath) {
return false, nil
}
Copy link
Member

Choose a reason for hiding this comment

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

🐍 question: Do we want to avoid reactivating a virtual environment if one is present? This might be an issue if different environments are activated for different reasons, but I'm unsure if that's a common practice.

I'm alright merging this without a clear answer to extend these approaches based on feedback! The happy path works so well and .venv appears to be convention without suggesting .venv.testing or similar.

if activated, err := runtime.ActivatePythonVenvIfPresent(c.Fs, dirPath); err != nil {
c.IO.PrintDebug(ctx, "failed to activate Python virtual environment: %s", err)
} else if activated {
c.IO.PrintDebug(ctx, "Activated Python virtual environment .venv")
Copy link
Member

Choose a reason for hiding this comment

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

⚠️ suggestion(non-blocking): Your approach is solid, but we might consider outputting the errors as a warning while keeping expected activations to the debug logs?

Would love to hear your thoughts on this! IIRC logic similar to what's here exists for the get-hooks activation, but we often miss problems that should be at least reported. Even if the command continues I think!

@zimeg zimeg merged commit 4f51fa6 into main Feb 27, 2026
8 checks passed
@zimeg zimeg deleted the mwbrooks-python-venv-activation branch February 27, 2026 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:bolt-python Related to github.com/slackapi/bolt-python changelog Use on updates to be included in the release notes enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants