Repository navigation
fix(cli): scope V2 commands to the caller's workspace, reject unknown flags (#143) - #144
Merged
Merged
Conversation
… flags (#143) Two CLI correctness problems, both of which made a probe or a query look like it had succeeded while describing someone else's window. WMUX_SURFACE_ID resolved to the wrong window -------------------------------------------- `windowForCaller` asked each window "do you hold this surface?" and stopped at the first yes. But `__wmux_hasSurface` scans every workspace in a window while `__wmux_getTree`/`listPanes`/`listSurfaces` answer about the *active* one — so resolution could be right about the window and still describe a workspace the caller had never seen. It now resolves the surface to its window AND its workspace (`__wmux_locateSurface`), and that workspace becomes the default for the methods that would otherwise guess. An explicit --workspace still wins. The window cache was also authoritative, so one outdated hit pinned a caller to that window for the rest of the session — the "byte-stable across focus changes" wrong answer in the report. It is now only a probe-order hint, so a stale entry heals on the next call. Underneath that, surface ids were not unique across windows. A window opened mid-run gets no session slot, which the renderer read as "nothing saved" and answered by cloning the newest *named* session — same workspace, pane and surface ids as the window that already had them. PTY id is surface id, so the clone's terminals re-attached to the original's PTYs, and every id-based lookup had two equally valid answers. `session:load-auto` now distinguishes "nothing saved" (null, launch windows only) from "come up empty" ({ fresh: true }), which is the contract per-window restore has claimed since issue #118. Id-targeted ops (close/focus/rename surface, close pane) now find their target anywhere in the window instead of only in the active workspace. Ids are uuids, so the search cannot hit the wrong thing — whereas the old scope silently did nothing and still reported ok. Unknown flags ran the command anyway ------------------------------------ `getFlag()` picked out the flags it knew and ignored the rest, so `wmux new-surface --help` created a surface and `wmux split --help` split a pane. Every command now declares its flags; anything else is rejected with that command's usage, and --help/-h prints usage without touching the pipe. A value flag with no value is rejected too — same silent default, different costume. Commands whose arguments are free-form text or belong to ssh are exempt (--help is part of what you are sending); `wmux help <command>` works for those. COMMANDS is typed against the spec table, so the compiler catches a command shipped without usage text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7nJeuxL4z9Yi5NL2KuhCV
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 524d23c37c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+760
to
+763
| if (i === args.length - 1 && arg !== '--blocked') { | ||
| fail(command, spec, `Flag '${arg}' needs a value.`); | ||
| } | ||
| i++; |
There was a problem hiding this comment.
When a value-taking flag is followed by another flag, this check only verifies that some token exists and then skips that token. For example, wmux split --type --bogus bypasses the unknown-flag rejection, after which getFlag() treats --bogus as the surface type and the command still mutates the layout. Treat a following --... token as a missing value before advancing, while handling the genuinely optional --blocked argument separately.
Useful? React with 👍 / 👎.
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.
Fixes #143.
1.
WMUX_SURFACE_IDresolved to the wrong windowwindowForCallerasked each window "do you hold this surface?" and stopped at the first yes. But__wmux_hasSurfacescans every workspace in a window, while__wmux_getTree/listPanes/listSurfacesanswer about the active one — so resolution could be right about the window and still describe a workspace the caller had never seen.Resolution now returns the surface's window and its workspace (
__wmux_locateSurface), and that workspace becomes the default for the methods that would otherwise guess. An explicit--workspacestill wins.The window cache was also authoritative, so one outdated hit pinned a caller to that window for the rest of the session — the "byte-stable across focus changes" wrong answer in the report. It is now only a probe-order hint, so a stale entry heals on the next call.
The duplicate ids underneath
Surface ids were not unique across windows. A window opened mid-run gets no session slot, which the renderer read as "nothing saved" and answered by cloning the newest named session — same workspace, pane and surface ids as the window that already had them. PTY id is surface id in wmux, so the clone's terminals re-attached to the original window's PTYs, and every id-based lookup had two equally valid answers.
session:load-autonow distinguishes "nothing saved" (null, launch windows only) from "come up empty" ({ fresh: true }) — the contract per-window restore has claimed since #118, now actually enforceable on the renderer side.Id-targeted operations (
close-surface,focus-surface,rename-surface,close-pane) also now find their target anywhere in the window instead of only in the active workspace. Ids are uuids, so the search cannot hit the wrong thing — whereas the old scope silently did nothing and still reportedok.2. Unknown flags ran the command anyway
getFlag()picked out the flags it knew and ignored the rest, sowmux new-surface --helpcreated a surface andwmux split --helpsplit a pane.Every command now declares its flags. Anything else is rejected with that command's usage, and
--help/-hprints usage without touching the pipe. A value flag with no value is rejected too — same silent default, different costume.Commands whose arguments are free-form text or belong to another program (
send,notify,log,ssh,browser,markdown,rename-*) are exempt, because--helpthere is part of what you are sending;wmux help <command>works for those.COMMANDSis typed against the spec table, so the compiler catches a command shipped without usage text.Verification
npx tsc --noEmitclean for both projects;npm run lintshows no new findings.tests/unit/caller-scope.test.tscovers wrong-window selection, background-workspace scoping, stale-cache healing, unknown-surface fallback and mid-probe window death;session-windows.test.tscoversrestoreAnswerFor.dist/cli/wmux.js(they exit before any pipe I/O).🤖 Generated with Claude Code
https://claude.ai/code/session_01Y7nJeuxL4z9Yi5NL2KuhCV