Handle stale MCP cleanup failures when switching workspaces - #770
Merged
Merged
Conversation
rohita5l
added a commit
that referenced
this pull request
Sep 21, 2026
## Summary Temporarily make the **User Journey Test Required** check advisory while GitHub-hosted runner access is blocked by the Databricks organization IP allowlist (observed on #770). - Add step-level `continue-on-error: true` to the existing judge invocation, so failures no longer fail the named job check. - Keep the judge running and preserve its error annotations and logs. This makes all judge-step failures advisory, including missing-coverage verdicts, until enforcement is restored. - Keep the workflow/check names, trusted-base checkout, `pull_request_target` trigger, permissions, secrets, and waiver workflow unchanged. This is a standalone three-line workflow change based on `main`; it does not include the code or CUJ changes from #770. It does not fix runner connectivity or change branch-protection settings. Remove the temporary tolerance once approved runner egress is available. ## Validation - Existing user-journey gate tests: **8 passed**. - YAML parse and semantic comparison: the only configuration change is step-level `continue-on-error: true`. - `git diff --check` passed. - Hosted execution has not been verified; `pull_request_target` uses the base-branch workflow, so this change takes effect only after landing on `main` and a fresh qualifying PR event. This PR's own check will still use the existing blocking workflow.
rohita5l
force-pushed
the
fix/workspace-mcp-cleanup
branch
from
September 21, 2026 01:53
a33ed4f to
4364e85
Compare
david-siqi-liu
approved these changes
Sep 21, 2026
david-siqi-liu
left a comment
Collaborator
There was a problem hiding this comment.
Approved to unblock, can you check if skills has the same issue?
rohita5l
enabled auto-merge (squash)
September 21, 2026 13:59
lilly-luo
approved these changes
Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switching workspaces carries the old skills MCP registration into the new workspace's state. Cleanup can then attempt the same removal twice, and a Claude CLI timeout or OS error escapes as a traceback after the stale-entry warning. Deduplicate removal attempts by client/server and report recoverable cleanup failures as warnings so configuration can continue.
Adds regression tests for both cleanup passes, continued cleanup of other clients, persistence of the new workspace, and preservation of the historical workspace bucket. The original screenshot omitted the final exception; these tests reproduce the confirmed code defects without claiming to identify that user's exact exception.
Adds an installed-product CUJ,
test_ug_configure_claude_cleans_stale_skills_mcp_on_workspace_switch: configure the first real workspace, register its skills MCP, switch to a second real workspace, verify removal throughclaude mcp getand saved state, repeat configure, and complete a real Claude file-reading task. No mocks, fabricated ug state, or injected managed configuration. The CUJ covers the normal transition; deterministic timeout coverage remains in unit/component tests.The runner accepts an explicit second workspace and bearer, redacts both credentials, and runs this CUJ in the existing Managed config · Claude lane using the two existing CI workspace credential sets. That lane retains its existing non-blocking policy.
Validation:
git diff --check, runner help, and both-workspace credential redaction checks passed.