Fix: Mise configuration changes causing blocking CI failures #196
Conversation
The mise-action was hanging during the "Exporting mise environment variables" step, causing the runner to kill the job with exit code 137 (SIGKILL). This was happening because: 1. The mise.toml has complex template expressions with command executions 2. The hooks.enter configuration may execute during env export 3. The env export was timing out after ~1 minute Solution: Disable the env parameter (set to false) in all mise-action configurations to prevent the hanging during environment variable export. Fixes #issue
…ZvD3Lo3vX7tdaw9xrKC' into fix-test-ci-action-failures
The previous fix disabled env export to prevent hanging, but this caused MISE_PATH to be empty because mise wasn't in the PATH. Changes: 1. Add mise bin directory to PATH in "Set outputs" step 2. Add mise bin directory to PATH in workflow steps that execute mise commands This ensures: - The "Set outputs" step can find mise using which - Workflow steps can execute mise commands via $MISE_PATH - No hanging during env export (env: false is still set) Fixes the "command not found" error when running mise commands.
…ZvD3Lo3vX7tdaw9xrKC' into fix-test-ci-action-failures
The composite action was setting outputs in the mise-outputs step, but those outputs were not being exposed by the action itself because the action.yml was missing the outputs section. Changes: - Add outputs section to action.yml defining all output variables - Map outputs from mise-outputs step to action outputs This ensures that steps.setup-mise.outputs.MISE_PATH and other outputs are properly available to the calling workflow. Related to the CI skip issue fix.
…ZvD3Lo3vX7tdaw9xrKC' into fix-test-ci-action-failures
Reviewer's GuideStandardizes and hardens CI workflows around Mise- and uv-based Python environments, simplifies MCP registry submission, and propagates environment details and flags through reusable workflows for more reliable, debuggable automation. Sequence diagram for MCP Registry submission workflowsequenceDiagram
actor Developer
participant GitHubActions as GitHub_Actions_Workflow
participant SetupUV as setup-uv-env_action
participant SubmitStep as Submit_to_MCP_Registry_step
participant OpenSSL as openssl_pkeyutl
participant MCPAuth as MCP_Registry_Auth_v0_1
participant MCPPub as MCP_Registry_Publish_v0_1
Developer->>GitHubActions: Push_release_or_dispatch
GitHubActions->>SetupUV: Setup UV and Python environment
SetupUV-->>GitHubActions: uv-path, uvx-path outputs
GitHubActions->>SubmitStep: Run submission shell script
SubmitStep->>SubmitStep: Create temp files (indata, key_file, outfile, request_body)
SubmitStep->>SubmitStep: Write MCP_REGISTRY_KEY PEM to key_file
SubmitStep->>SubmitStep: Generate timestamp (RFC3339)
SubmitStep->>SubmitStep: Write timestamp to indata
SubmitStep->>OpenSSL: Sign timestamp with ed25519 key
OpenSSL-->>SubmitStep: Signature bytes
SubmitStep->>SubmitStep: Hex-encode signature (signed_timestamp)
SubmitStep->>SubmitStep: Build auth JSON body {domain, signed_timestamp, timestamp}
SubmitStep->>MCPAuth: POST /v0.1/auth/dns with auth JSON
MCPAuth-->>SubmitStep: registry_token JSON
SubmitStep->>SubmitStep: Extract registry_token with jq
SubmitStep->>MCPPub: POST /v0.1/publish with server.json and Bearer registry_token
MCPPub-->>SubmitStep: Publication response JSON
SubmitStep->>SubmitStep: Extract published_at from _meta.io.modelcontextprotocol.registry/official
SubmitStep-->>GitHubActions: MCP_PUBLICATION_TIME output
GitHubActions-->>Developer: Step summary with version, status, published_at URL
Flow diagram for setup-mise-env composite action and free-threaded Python handlingflowchart TD
Start["Start setup-mise-env"] --> Checkout["Checkout repository (optional)"]
Checkout --> FreeDisk["Free up disk space script"]
FreeDisk --> SysDeps["Install system dependencies (curl, git, build libs)"]
SysDeps --> SetFTEnv["Set up Environment for Free-Threaded Python"]
SetFTEnv --> CheckFT{Ends with 't'?}
CheckFT -- No --> StdPy["Use standard Python version\nMISE_PYTHON_VERSION=inputs.python-version\nUV_PYTHON=inputs.python-version"]
CheckFT -- Yes --> FTPy["Free-threaded Python requested\nMISE_PYTHON_VERSION=base_version\nUV_PYTHON=base_version+freethreaded"]
StdPy --> ProfileDecision
FTPy --> ProfileDecision
ProfileDecision{Profile}
ProfileDecision -- minimal --> MiseMinimal["Setup Mise profile=minimal\n(env: false, cache, install)"]
ProfileDecision -- reviewer --> MiseReviewer["Setup Mise profile=reviewer\n(MISE_ENV=dev, env: false)"]
ProfileDecision -- dev --> MiseDev["Setup Mise profile=dev\n(MISE_ENV=dev, env: false)"]
MiseReviewer --> ActivateReviewer["Activate Reviewer Environment\nuv sync and mise commands"]
MiseDev --> MaybeInsertFT{Free-threaded?}
MaybeInsertFT -- No --> ActivateDev
MaybeInsertFT -- Yes --> InsertFT["Insert Free-Threaded Python into Dev Profile\nuse uv to install cpython-UV_PYTHON\nrun mise sync python --uv"] --> ActivateDev
ActivateDev["Activate Dev Profile Environment\n- mise activate bash --shims\n- mise doctor\n- mise use -g uv@latest\n- mise //:cloud-setup"]
MiseMinimal --> Outputs
ActivateReviewer --> Outputs
ActivateDev --> Outputs
Outputs["Set outputs (MISE_PATH, MISE_ENV, PROFILE, PYTHON_PATH, MISE_PYTHON_VERSION, bin dirs)"] --> End["End setup-mise-env"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
👋 Hey @bashandbone, Thanks for your contribution to codeweaver! 🧵You need to agree to the CLA first... 🖊️Before we can accept your contribution, you need to agree to our Contributor License Agreement (CLA). To agree to the CLA, please comment:
Those exact words are important1, so please don't change them. 😉 You can read the full CLA here: Contributor License Agreement ✅ @bashandbone has signed the CLA. 0 out of 3 committers have signed the CLA. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
copilot-setup-steps.yml, theVerify Mise Setupcommandmise --version || echo "Mise not in path" || "$MISE_PATH" --versionwill never reach the final"$MISE_PATH" --versionbecauseechoalways succeeds; consider restructuring withif/elseor a single||chain so the fallback actually runs. - You’re hardcoding
$HOME/.local/share/mise/binin multiple workflow steps while also exposingMISE_PATHfrom the composite action; consider deriving the bin/shim paths from the action outputs (or centralizing PATH setup) to avoid future drift if Mise’s install location changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `copilot-setup-steps.yml`, the `Verify Mise Setup` command `mise --version || echo "Mise not in path" || "$MISE_PATH" --version` will never reach the final `"$MISE_PATH" --version` because `echo` always succeeds; consider restructuring with `if`/`else` or a single `||` chain so the fallback actually runs.
- You’re hardcoding `$HOME/.local/share/mise/bin` in multiple workflow steps while also exposing `MISE_PATH` from the composite action; consider deriving the bin/shim paths from the action outputs (or centralizing PATH setup) to avoid future drift if Mise’s install location changes.
## Individual Comments
### Comment 1
<location> `.github/workflows/copilot-setup-steps.yml:79` </location>
<code_context>
+ MISE_EXPERIMENTAL: 1
+ MISE_PATH: ${{ steps.setup-mise.outputs.MISE_PATH }}
+ run: |
+ mise --version || echo "Mise not in path" || "$MISE_PATH" --version
</code_context>
<issue_to_address>
**issue (bug_risk):** The fallback to use MISE_PATH will never run because the echo command short-circuits the `||` chain.
Because `echo` returns 0, a chain like `mise --version || echo ... || "$MISE_PATH" --version` will always stop at the `echo` and never reach the fallback. To actually run the explicit path when the PATH lookup fails, group the message and fallback together, e.g.:
```bash
mise --version || { echo "Mise not in path"; "$MISE_PATH" --version; }
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR fixes CI-breaking workflow issues caused by changes in Mise configuration and setup. It enhances Python environment management using Mise and improves workflow reliability through better environment configuration and explicit outputs.
Key Changes:
- Added structured outputs to the
setup-mise-envaction (MISE_PATH, MISE_ENV, PROFILE, PYTHON_VERSION, PYTHON_PATH) for better environment visibility - Disabled automatic environment export in Mise setup steps and explicitly added Mise to PATH for consistent configuration
- Updated test workflows to use Mise-based Python setup with improved PATH handling for quality checks and test execution
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/actions/setup-mise-env/action.yml |
Added outputs for environment details, disabled automatic env export, explicitly configured PATH in output step |
.github/workflows/copilot-setup-steps.yml |
Added Mise environment setup and verification steps for Python 3.13 |
.github/workflows/ci.yml |
Added workflow parameters for test markers, coverage upload, and quality checks; updated comment for clarity |
.github/workflows/_reusable-test.yml |
Updated step names for clarity, fixed PATH configuration to include Mise binaries, removed unnecessary flags from experimental Python setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bashandbone
changed the title
Co-authored-by: Copilot <[email protected]> Signed-off-by: Adam Poulemanos <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@bashandbone I've opened a new pull request, #197, to work on those changes. Once the pull request is ready, I'll request review from you. |
Remove the first instance of "Setup Mise Environment" step (lines 52-58) as it was a duplicate. The second instance with id: setup-mise is kept since it's needed for the Verify Mise Setup step that references it. Co-authored-by: bashandbone <[email protected]>
Remove duplicate Setup Mise Environment step in copilot-setup-steps workflow
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "mise trust > NUL 2>&1", | ||
| "mise install > NUL 2>&1 || echo.", | ||
| "hk install --mise > NUL 2>&1 || echo.", | ||
| "mise trust --all > NUL 2>&1", |
There was a problem hiding this comment.
The mise trust --all command is potentially dangerous as it trusts all mise configurations without user confirmation. The --all flag trusts configurations in all parent directories up to the root, which could inadvertently trust malicious configurations in shared or system directories.
Consider using mise trust without the --all flag to only trust the current project's configuration, or add additional safeguards to verify the configuration source before trusting.
| "mise trust &>/dev/null", | ||
| "mise install &>/dev/null || true", | ||
| "hk install --mise &>/dev/null || true", | ||
| "mise trust --all &>/dev/null", |
There was a problem hiding this comment.
The mise trust --all command is potentially dangerous as it trusts all mise configurations without user confirmation. The --all flag trusts configurations in all parent directories up to the root, which could inadvertently trust malicious configurations in shared or system directories.
Consider using mise trust without the --all flag to only trust the current project's configuration, or add additional safeguards to verify the configuration source before trusting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # profile is set to 'dev' in a cloud CI environment for cloud agents. 'enter' tends to break there. | ||
| if [ "$profile" = "dev" ] && [ "$CI" = "true" ]; then | ||
| # don't run | ||
| exit 0 |
There was a problem hiding this comment.
[nitpick] The comparison operator uses == which is a bash-specific feature. For POSIX compatibility, consider using = instead. However, since the shebang is likely bash, this is acceptable but inconsistent with line 177 in mise.toml which uses =. Consider standardizing the comparison operator across the codebase.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Adam Poulemanos <[email protected]>
bashandbone
left a comment
bashandbone
left a comment
There was a problem hiding this comment.
Well folks. This was two days of absolute misery. Someone remind me of this next time I consider so much as blinking at CI/CD.
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- The new MCP registry submission step assumes
jq,xxd,shred, and recentopenssl pkeyutlsupport are present on the runner; consider either installing/validating these explicitly in the workflow or adding graceful error messages if they are missing or incompatible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new MCP registry submission step assumes `jq`, `xxd`, `shred`, and recent `openssl pkeyutl` support are present on the runner; consider either installing/validating these explicitly in the workflow or adding graceful error messages if they are missing or incompatible.
## Individual Comments
### Comment 1
<location> `mise.dev.toml:86-87` </location>
<code_context>
+ fi
+ fi''',
+ "echo \"[codeweaver] Using mise...\"",
+ '''if command -v mise &>/dev/null && ! grep -q "mise activate" "$HOME/.bashrc"; then
+ echo "eval \"$(mise activate bash)\" 2>&1 || true" >> "$HOME/.bashrc"
+ fi''',
+ '''if command -v mise &>/dev/null && ! grep -q "mise activate" "$HOME/.bash_profile"; then
</code_context>
<issue_to_address>
**issue (bug_risk):** The `mise activate` command is evaluated during setup instead of at shell startup, and the resulting line in `.bashrc` may be malformed.
Because `$(mise activate bash)` is inside double quotes, it runs during `cloud-setup`, and its (likely multi-line) output is inlined into the `echo`, producing a malformed one-line `eval "..." 2>&1 || true` in `.bashrc`.
To append the literal command for later execution at shell startup, use single quotes so the substitution isn’t evaluated at setup time, e.g.:
```bash
echo 'eval "$(mise activate bash --shims)" 2>/dev/null || true' >> "$HOME/.bashrc"
```
Apply the same pattern to the `.bash_profile` block below.
</issue_to_address>
### Comment 2
<location> `.github/workflows/mcp-registry-submit.yml:160-165` </location>
<code_context>
+ echo -n '"timestamp":"'"$timestamp"'"}'
+ } > "$request_body"
+ echo "Logging in to MCP Registry..."
+ response="$(curl --request POST \
+ --url "https://registry.modelcontextprotocol.io/v0.1/auth/dns" \
+ --header "Accept: application/json, application/problem+json" \
+ --header "Content-Type: application/json" \
+ --data @"$request_body")"
+ token="$(echo "$response" | jq -r '.registry_token')"
+ if [ -z "$token" ] || [ "$token" = "null" ]; then
+ echo "ERROR: Failed to obtain registry token"
</code_context>
<issue_to_address>
**issue (bug_risk):** The registry auth/publish steps assume `jq` is available on the runner, which may not always hold.
Both login and publish parse responses with `jq`, but the workflow never ensures it is installed. While `ubuntu-latest` currently includes `jq`, that’s not guaranteed across all runners or over time, and missing `jq` would cause the registry token/timestamp extraction to fail. Please add an explicit install/verification step early in the job so the workflow doesn’t depend on runner defaults.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| '''if command -v mise &>/dev/null && ! grep -q "mise activate" "$HOME/.bashrc"; then | ||
| echo "eval \"$(mise activate bash)\" 2>&1 || true" >> "$HOME/.bashrc" |
There was a problem hiding this comment.
issue (bug_risk): The mise activate command is evaluated during setup instead of at shell startup, and the resulting line in .bashrc may be malformed.
Because $(mise activate bash) is inside double quotes, it runs during cloud-setup, and its (likely multi-line) output is inlined into the echo, producing a malformed one-line eval "..." 2>&1 || true in .bashrc.
To append the literal command for later execution at shell startup, use single quotes so the substitution isn’t evaluated at setup time, e.g.:
echo 'eval "$(mise activate bash --shims)" 2>/dev/null || true' >> "$HOME/.bashrc"Apply the same pattern to the .bash_profile block below.
| response="$(curl --request POST \ | ||
| --url "https://registry.modelcontextprotocol.io/v0.1/auth/dns" \ | ||
| --header "Accept: application/json, application/problem+json" \ | ||
| --header "Content-Type: application/json" \ | ||
| --data @"$request_body")" | ||
| token="$(echo "$response" | jq -r '.registry_token')" |
There was a problem hiding this comment.
issue (bug_risk): The registry auth/publish steps assume jq is available on the runner, which may not always hold.
Both login and publish parse responses with jq, but the workflow never ensures it is installed. While ubuntu-latest currently includes jq, that’s not guaranteed across all runners or over time, and missing jq would cause the registry token/timestamp extraction to fail. Please add an explicit install/verification step early in the job so the workflow doesn’t depend on runner defaults.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
.vscode/terminal.extra.zsh:40
- [nitpick] Inconsistent indentation in the else block. Lines 33-40 have mixed indentation levels - the
chmodcommand and subsequent lines are indented with spaces, creating a confusing code structure.
The code should have consistent indentation:
else
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || echo "${PWD}")"
# Mise is not installed, run the install script
chmod -R +x "${REPO_ROOT}/scripts" || {
print -P "%F{209}[codeweaver]%f %F{red}Failed to make install scripts executable!%f"
}
print -P "%F{209}[codeweaver]%f Installing Mise for the first time..."
"${REPO_ROOT}/scripts/install-mise.sh" || {
print -P "%F{209}[codeweaver]%f %F{red}Failed to install Mise!%f Try running the install script manually: %F{cyan}${REPO_ROOT}/scripts/install-mise.sh%f"
}
full_setup
fielse
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || echo "${PWD}")"
# Mise is not installed, run the install script
chmod -R +x "${REPO_ROOT}/scripts" || {
print -P "%F{209}[codeweaver]%f %F{red}Failed to make install scripts executable!%f"
}
print -P "%F{209}[codeweaver]%f Installing Mise for the first time..."
"${REPO_ROOT}/scripts/install-mise.sh" || {
print -P "%F{209}[codeweaver]%f %F{red}Failed to install Mise!%f Try running the install script manually: %F{cyan}${REPO_ROOT}/scripts/install-mise.sh%f"
}
full_setup
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # after a lot of trial and error, it seems that combined with setting MISE_PYTHON_VERSION to the base version, and using uv to install the free-threaded version, and syncing uv and mise, this works | ||
| # the favored version is now the free-threaded version -- as mise list python will show: |
There was a problem hiding this comment.
[nitpick] The comment spans two lines and could be more concise. Long comments in shell scripts embedded in YAML can be hard to read. Consider breaking it into multiple single-line comments or shortening it:
# After trial and error: set MISE_PYTHON_VERSION to base version, install free-threaded via uv, then sync.
# The favored version is now free-threaded -- visible in: mise list python| # after a lot of trial and error, it seems that combined with setting MISE_PYTHON_VERSION to the base version, and using uv to install the free-threaded version, and syncing uv and mise, this works | |
| # the favored version is now the free-threaded version -- as mise list python will show: | |
| # Set MISE_PYTHON_VERSION to base version, install free-threaded via uv, then sync. | |
| # After these steps, the favored version is free-threaded (see: mise list python). |
| function _mise_hook { | ||
| local diff=${__MISE_DIFF} | ||
| source <(command mise hook-env -s zsh) | ||
| [[ ${diff} == ${__MISE_DIFF} ]] | ||
| _mise_updated=$? | ||
| } |
There was a problem hiding this comment.
The _mise_hook and _prompt functions are defined but _mise_hook is never called in this file. The hook may be intended to be registered elsewhere, but without that registration visible here, the _mise_updated variable will never be set, meaning the prompt function will always use the default behavior (green/red based on exit status) rather than the blue prompt for mise updates.
Consider either:
- Adding the hook registration:
add-zsh-hook precmd _mise_hook - Or documenting where/how this hook is meant to be registered
| domain="knitli.com" | ||
| timestamp="$(date -u +"%Y-%m-%dT%H:%M:%SZ")" | ||
| echo -n "$timestamp" > "$indata" | ||
There was a problem hiding this comment.
The secret key is being written to a temporary file without verification that the file permissions are restrictive enough before writing. While umask 077 is set, there's a race condition where an attacker could potentially access the file between creation and the shred operation.
Consider using a more secure approach:
umask 077
key_file="$(mktemp)"
chmod 600 "$key_file"
echo "$MCP_REGISTRY_KEY" > "$key_file"This ensures the file has restrictive permissions immediately after creation.
| chmod 600 "$key_file" |
| UV_PYTHON: ${{ inputs.python-version }} | ||
| UV_PYTHON: ${{ env.UV_PYTHON }} | ||
| MISE_VERBOSE: ${{ inputs.mise-debug }} | ||
| MISE_PYTHON_COMPILE: 0 |
There was a problem hiding this comment.
[nitpick] Setting MISE_PYTHON_COMPILE=0 disables Python compilation which may impact performance for compute-intensive operations. While this speeds up installation in CI, it means tests run on unoptimized Python builds.
Consider documenting this trade-off in a comment, or making it configurable:
# Disable Python compilation for faster CI setup (trade: slower runtime)
MISE_PYTHON_COMPILE: 0This helps future maintainers understand the performance implications.
| '''bash -c " | ||
| ignore_files=('src/codeweaver/_version.py' 'coverage.xml' 'test-results.xml') | ||
| for file in \"\${ignore_files[@]}\"; do | ||
| if ! grep -qx \"\$file\" .git/info/exclude; then |
There was a problem hiding this comment.
The grep -qx in line 103 uses the -x flag which matches the entire line exactly. However, the .git/info/exclude file may have leading/trailing whitespace or different formatting. This could cause duplicate entries to be added.
Consider using a more robust check:
if ! grep -qF "$file" .git/info/exclude 2>/dev/null; then
echo "$file" >> .git/info/exclude 2>&1
fiThe -F flag treats the pattern as a fixed string (not regex) and is more reliable for this use case.
| if ! grep -qx \"\$file\" .git/info/exclude; then | |
| if ! grep -qF \"\$file\" .git/info/exclude; then |
PR Review: CI Workflow ImprovementsI've completed a comprehensive review of PR #196. This is an extensive and well-executed improvement to the CI/CD infrastructure. Below is my detailed feedback: ✅ Strengths1. Comprehensive Mise IntegrationThe integration of Mise for environment management is well-designed:
2. Workflow Simplification
3. MCP Registry Submission
4. Documentation & Comments
🔴 Critical Issues1. Missing
|
| Metric | Assessment |
|---|---|
| Code Style | ✅ Excellent (consistent YAML formatting, proper indentation) |
| Documentation | ✅ Good (inline comments, clear step names) |
| Security | |
| Maintainability | ✅ Good (modular design, reusable workflows) |
| Testing |
🔧 Recommended Actions (Priority Order)
- Critical: Add explicit
jqinstallation or switch to Python JSON parsing - High: Investigate and resolve CI test failures (especially experimental Python versions)
- Medium: Add validation for free-threaded Python installation
- Medium: Improve error handling for
continue-on-errorsteps - Low: Add disk space logging to cleanup script
- Low: Document temporary files cleanup behavior
📝 Summary
This PR represents a significant improvement to the CI/CD infrastructure with excellent attention to detail in most areas. The Mise integration is well-executed, and the MCP registry submission improvements are solid. The main concerns are:
- The
jqdependency issue (easily fixed) - Current CI failures (need investigation)
- Free-threaded Python workaround (needs documentation/tracking)
Recommendation: Address the jq dependency and investigate the CI failures before merging. Once those are resolved, this PR will be ready for approval.
Overall Assessment: 👍 Approve with changes requested
Let me know if you'd like me to help address any of these issues!
PR Review: CI/CD Workflow and Mise Configuration UpdatesI've reviewed PR #196 which updates the GitHub Actions workflows to fix CI-breaking issues and improve Python environment management with Mise. This is a substantial refactor with 19 files changed and 484 additions. Here's my comprehensive feedback: ✅ Strengths
|
This pull request updates the GitHub Actions workflows as a fix to a CI-breaking workflow caused by changes in mise configuration and setup. It also improves how Python environments are set up and managed using Mise, and enhances workflow outputs and environment configuration for better reliability and maintainability. The main changes are grouped below by theme.
Mise Environment Setup & Workflow Outputs
setup-mise-envaction, includingMISE_PATH,MISE_ENV,PROFILE,PYTHON_VERSION, andPYTHON_PATH, making it easier for subsequent steps to access environment details.env: false) in all Mise setup steps, and explicitly added Mise to thePATHin output steps to ensure consistent environment configuration. [1] [2] [3]Workflow Improvements & Reliability
PATHincludes Mise binaries for quality checks and test runs. [1] [2] [3] [4]uv syncfor cleaner dependency resolution.CI/CD Configuration Enhancements
test-markers,upload-coverage,run-quality-checks) to the test workflow from the main CI workflow for more flexible and granular control.Copilot Setup Workflow
copilot-setup-steps.ymlworkflow, ensuring Python 3.13 is correctly installed and Mise is available in the path.Note
Summary by Sourcery
Update CI workflows and mise-based environment setup for Python tooling, MCP registry publishing, and automation reliability.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
Chores: