Repository navigation
fix(cli): fail the Pi feed when an explicit workspace ref stays unresolved - #14277
Merged
Merged
Conversation
…olved Since #13964, resolveWorkspaceId returns an unmatched ref unchanged when it cannot scan every window (a failed window.list, or a relay). The Pi feed took that as resolved, dropped the non-UUID scope, and routed the event by surface alone, so an event for a missing --workspace was delivered to whatever workspace owns the surface. The feed now fails with its unavailable-target exit code (69) unless an explicit selector resolves to a UUID. test_pi_feed_rejects_missing_explicit_workspace caught this once the CLI regression shard got past its earlier failures (#14246, #14263). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe Pi hook target resolver now rejects an explicitly supplied workspace selector when it does not resolve to a UUID. It throws the surface-not-found error instead of continuing with surface-only routing. ChangesPi hook routing
Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Unresolved explicit workspace selectors now return the unavailable-target error without routing by surface alone. The inspected change leaves resolved UUID selectors and full-scan no-match behavior unchanged, with no actionable merge risk identified. 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
teamleaderleo
enabled auto-merge (squash)
September 24, 2026 19:40
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 24, 2026
2ef659a fix(cli): fail the Pi feed when an explicit workspace ref stays unresolved (manaflow-ai#14277) 2d821f1 Track per-hop mobile keystroke latency and pacer rate in the existing Axiom window (manaflow-ai#14270)
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
1 participant
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.
tests/test_pi_compacted_feed.pyfails on the CLI regression shard: "Pi feed did not report its missing explicit workspace as an unavailable target". The test was right: the Pi feed leaks events across workspace scope.Cause
Since #13964,
resolveWorkspaceIdreturns an unmatched ref unchanged when it cannot scan every window, for example whenwindow.listfails, or over a relay, where the host resolves the ref. The Pi feed treated that raw ref as a resolution. Because the ref isn't a UUID, it leftworkspace_idout ofagent.resolve_delivery_targetand routed by surface alone. The failing run's frames show the whole sequence:workspace.listwindow.listreturns not_foundagent.resolve_delivery_targetis called with onlysurface_idfeed.pushsends the event to workspace6666…, which is not the requestedworkspace:404Fix
After resolution, an explicit
--workspaceselector must end as a UUID. If it doesn't, the feed throws its existing unavailable-target error (piHookSurfaceNotFoundError, exit 69, v2not_found). This follows the file's own rule: a supplied index or ref "cannot be discarded without violating the caller's explicit scope".Behavior change: over a relay,
cmux hooks feed --source pi --workspace workspace:Nnow fails with 69 instead of silently delivering by surface. This also covers relay runs that pass a ref workspace together with an index or ref surface, or with no surface. Those used to send the ref throughsurface.listfor the host to resolve, and now fail with 69 too. The guard runs in the shared target resolution, so Pi hook events (cmux hooks pi ..., throughresolveStrictPiHookTarget) get the same behavior ashooks feed. A caller that passes a workspace UUID is unaffected, andCMUX_WORKSPACE_IDis always a UUID.Not changed: a full scan that finds no match throws
Workspace ref not foundwithout a v2 code, so it exits 1, not 69. That was already true before this PR. Fixing it would mean givingresolveWorkspaceId's errors anot_foundcode, which also changes error output for its 26 other callers, so it's left for a separate change.Validation
The CLI regression shard (app-host 4/7) runs
tests/test_pi_compacted_feed.py, includingtest_pi_feed_rejects_missing_explicit_workspace. I did not build the CLI locally (no local cmux builds on this machine), so CI is the proof.🤖 Generated with Claude Code
Summary by CodeRabbit