Skip to content

Handle stale MCP cleanup failures when switching workspaces - #770

Merged
rohita5l merged 2 commits into
mainfrom
fix/workspace-mcp-cleanup
Sep 21, 2026
Merged

rohita5l merged 2 commits into
mainfrom
fix/workspace-mcp-cleanup

Conversation

@rohita5l

Copy link
Copy Markdown
Collaborator

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 through claude mcp get and 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:

  • Before the fix: six focused failures exposed duplicate attempts and uncaught timeout/missing-executable errors.
  • After the fix: 805 targeted unit/component tests passed with inherited routing/workspace settings isolated.
  • After CUJ wiring: 16 regression/contract tests passed; the new CUJ and its full CI lane collect successfully.
  • Ruff lint/format checks, git diff --check, runner help, and both-workspace credential redaction checks passed.
  • Live CUJ execution is pending CI; collection is not a live pass.

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
rohita5l force-pushed the fix/workspace-mcp-cleanup branch from a33ed4f to 4364e85 Compare September 21, 2026 01:53
@rohita5l rohita5l added the ug0.2-launch-blocker Launch Blocker for UG 0.2 label Sep 21, 2026

@david-siqi-liu david-siqi-liu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved to unblock, can you check if skills has the same issue?

@rohita5l
rohita5l enabled auto-merge (squash) September 21, 2026 13:59
@rohita5l
rohita5l merged commit 3247280 into main Sep 21, 2026
22 checks passed
@rohita5l
rohita5l deleted the fix/workspace-mcp-cleanup branch September 21, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ug0.2-launch-blocker Launch Blocker for UG 0.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants