Repository navigation
feat(stagedctl): add project open to show a project from the terminal - #1072
Merged
Merged
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Step 1 of the `stagedctl project open` plan: the upcoming control command needs to know which window a reveal lands on before raising it, so it can address a request to that window. `show_a_window` did both in one go. Move it into a new `app_lifecycle::window_raise` submodule and split it: - `pick_window_to_show` chooses the window in the existing Dock-click order: focused, then visible, then `main`, then any. - `raise_window` shows, unminimizes and focuses a window and restores its PR-poll focus hint. - `show_a_window` is now the two run back to back and is re-exported from `app_lifecycle`, so the Dock reopen and `Window > Staged` callers are unchanged in both call site and behaviour. `set_native_focus` becomes generic over the runtime so the raise helper can run on tauri's mock runtime. The pick order is tested with mock-runtime windows. The mock runtime reports every window as visible and none as focused, so the order lives in a small function that takes the two probes, and the tests supply their own to reach each tier. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 2 of the `stagedctl project open` plan. The backend cannot switch a window's project itself: each window owns its selection, and only it knows whether a dialog is open or a diff comment still needs saving. So the upcoming control command needs somewhere to park a request for the window it picked, and a way for that window to say what it did. Add `project_open`, an in-memory state managed on the app: - `ProjectOpenState::request` parks a request for a window label and returns a handle its caller waits on. A window holds at most one: a newer request supersedes the older, whose caller reads `Superseded`. - A request lives only while its handle does. A wait that times out, or a caller that goes away, withdraws it, so a window that loads later never navigates for a caller already told it failed, and nothing needs pruning. - Outcomes are sent under the state's lock. A caller whose timeout fires as an answer lands therefore reads the answer the window was told had been recorded, not `TimedOut`. Add the window's side in `project_open/commands.rs`: - `list_pending_project_opens` is the snapshot a window takes once its listener is live, so a request sent before the window loaded is not lost. It mirrors the consent dialog's event-plus-snapshot pairing. - `answer_project_open` reports opened, dialog open, diff comment unsaved, or project not found. Both act on the calling window's label only, are refused by the web dispatch, and stay out of the stagedctl allowlist. Nothing parks a request yet; the control command that does is the next step. The state and the commands are tested for answer, supersede, timeout and withdrawal, and for window scoping on tauri's mock runtime. Nothing is persisted and there is no schema change, but this does add backend types (`ProjectOpenRequest`, `ProjectOpenAnswer`, `ProjectOpenOutcome`), which AGENTS.md asks a human to review. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 3 of the `stagedctl project open` plan: the command the CLI verb
will call. Steps 1 and 2 added the window pick and raise helpers and the
pending-open state; this joins them behind the control server.
`project_open::control::open_project` is the body of a new `open_project`
dispatch arm. Given a `projectId` it:
- checks the project exists before any window is touched, so a typo does
not pull Staged to the front;
- picks the window a Dock click would land on (`pick_window_to_show`);
- parks a request for that window in `ProjectOpenState`, emits
`project-open-requested` to that window alone, and raises it
(`raise_window`);
- waits up to `ANSWER_TIMEOUT` for the window's answer.
It returns `{ "outcome", "window" }` whatever the window did. The outcome
is `opened`, `dialogOpen`, `diffCommentUnsaved`, `projectNotFound`,
`superseded` or `timedOut`. A refusal is a result rather than an error
because the control server's plain errors carry no code, and the CLI needs
to tell these apart to word `ui_busy` and `ui_timeout`. Only a request
that could not be made, an unknown project or no window left, is an error.
Who may call it, using the plan's recommended defaults:
- Allowlisted in `ALLOWED_COMMANDS`, with a `CONTROL_SCOPE` row keyed on
`projectId`, so a project-scoped grant opens only its own project.
- Read-write: the window marks the project read and persists it as the
last one shown, so a read-only grant is refused.
- Session credentials are refused, even for the session's own project, so
an agent cannot pull the user's window away. This is a new `ScopeKey`,
`OpenProject`, and the first scope rule decided by the grant's actor;
the `Actor` and module docs say so.
- Web callers are refused in `web_caller_rejection`, ahead of the arm.
No protocol version bump: an older app answers 404 for the unknown
command, as it does for any other.
Tested: the open flow on tauri's mock runtime (answer, each refusal, only
the picked window told, unknown project, no window, timeout, supersede),
the scoped, read-only and session refusals in the scope table, the web
refusal, and the pinned allowlist.
Nothing listens for the event yet, so until the frontend steps land a call
raises the window and reads `timedOut`. No schema change and no new
backend type.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 4 of the `stagedctl project open` plan: what a window does with a request to show a project that came from outside it. Steps 1 to 3 added the backend half, which parks a request for one window and waits for that window's answer; this is the navigation the answer reports on. `openProjectExternally(projectId)` in `navigation.svelte.ts` resolves to a `ProjectOpenAnswer`: `opened`, `dialogOpen`, `diffCommentUnsaved` or `projectNotFound`. Those are the backend's `ProjectOpenAnswer` values as serde reads them, so step 5 can hand the result to `answer_project_open` unchanged. `selectProject` trusts its caller to have picked a safe moment. A request from outside can land at any time, so the helper makes the checks itself: - It waits for the first project list, since the app may have been launched for the request and the window still be loading. - It refuses with `dialogOpen` while a dialog or alert dialog is open. Navigating would unmount the view that owns the dialog along with whatever was typed into it. The command palette counts here. - Over the diff view it awaits `diffLeaveCheck`, as the palette does, and stays with `diffCommentUnsaved` when the save failed or threw. - It validates the id against the window's project list, treating a project with a delete in flight as not found, then calls `selectProject`. Three choices the plan did not spell out: - A project the window cannot show is `projectNotFound` ahead of the dialog and diff checks, and is checked again after a save. The caller is then never told to close a dialog and retry a request that can only fail. - A project that is already the view on screen is `opened` even under a dialog, and nothing is navigated or persisted again. This matches the plan's "already on screen: raise the window and exit 0". - A project list that failed to load refuses nothing, as for a new window's seed in `initNavigation`. The backend has already checked the id against the database. The dialog and the project are checked again after a diff save, because the save takes time. Tested in `navigation.test.ts`: each outcome, the already-on-screen and Settings cases, a dialog with no `data-state` and one animating out, a save that fails or throws, a dialog opening and a project deleted during the save, the wait for the first list, and a list that failed to load. Nothing calls the helper yet; the listener and the command bindings are step 5. Frontend only, with no backend change. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 5 of the `stagedctl project open` plan: the listener that joins the
two halves already on the branch. The backend parks a request for one
window, emits `project-open-requested` to it and waits for an answer; the
frontend has `openProjectExternally`, which navigates and says what
happened. Nothing connected them, so the `open_project` control command
raised the window and then read `timedOut`.
`listenForProjectOpens` in `listeners/projectOpenListener.ts` hands each
request to `openProjectExternally` and reports the result through
`answer_project_open`:
- Window-scoped, like the menu listener. An any-target listener also
matches emits addressed to other windows, so every window would
navigate.
- A request parked before the window was listening is still picked up.
Once the registration lands, the listener takes
`list_pending_project_opens`, the consent dialog's event-plus-snapshot
pairing.
- A request is never acted on or answered twice. The snapshot can hold a
request the event also delivered, so each request id is taken once.
- Requests run one at a time and the newest wins. One can wait on the
project list or on a diff comment being saved; a later one waits its
turn, so the window ends on the project asked for last. Only the newest
waiting request is kept, since the backend holds one per window and has
already told a replaced one's caller it was superseded.
- An answer the backend refuses, because the caller timed out or was
superseded meanwhile, is logged and dropped. If the open itself throws,
no answer is sent and the caller reads a timeout.
Supporting changes:
- `listenToWindowEvent` takes the same `ListenOptions` as `listenToEvent`
and calls `onEstablished` and `onRegistrationFailed` under the same
rules. This is the established hook the snapshot needs. The menu
listener passes none and is unchanged.
- `commands.ts` gains `listPendingProjectOpens` and `answerProjectOpen`,
and `types.ts` gains `ProjectOpenRequest`, mirroring the backend's
`{ requestId, projectId }`.
- `App.svelte` starts the listener in every desktop window, since the
backend picks whichever one a Dock click would raise. Web mode is
skipped: the web dispatch refuses both commands.
One choice the plan did not spell out: the listener starts after the
route restore (`initNavigation`) rather than beside the other listeners.
A request handled during the restore could be overwritten by it, most
plainly in a secondary window, which sets its seeded project
unconditionally. Starting late loses nothing, because the snapshot offers
any request parked earlier.
Tested in `projectOpenListener.test.ts` on the `menuListener.test.ts`
pattern: the event name and scope, the answer and each refusal, the
snapshot taken only once the listener is live, the event and snapshot
overlapping in either order, a repeated event, overlapping requests, a
refused answer, a failed open, a failed snapshot, and teardown.
`transport.test.ts` covers the new hooks on the window listener.
Not run against a live app: the unit tests mock the transport, so the
event and command round trip with the real backend is untested here.
Frontend only, with no Rust change.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 6 of the `stagedctl project open` plan: the CLI verb. Steps 1 to 5
added the app's half, an `open_project` control command that raises the
window a Dock click would land on, asks it to switch project, and answers
`{ "outcome", "window" }` whatever the window did. Nothing on the command
line called it.
`stagedctl project open <project>` resolves the name or id with the
existing rules (exact id, exact name, unique case-insensitive name), so
`not_found` and `ambiguous` are decided before any window is raised, then
sends the id alone to `open_project` and maps the outcome:
- `opened` exits 0 and prints the project's name and id and the pid of
the Staged instance. `--json` gives `{ outcome, project: { id, name },
window, pid }`.
- `dialogOpen` and `diffCommentUnsaved` exit 1 with `ui_busy`, each with
its own message. The dialog one is the plan's wording.
- `timedOut` exits 1 with `ui_timeout`.
- `projectNotFound`, a project the window lost between the CLI's lookup
and the switch, exits 1 with `not_found`.
- `superseded` exits 1 with `ui_busy` and a message of its own. The plan
gives it no code. `ui_busy` is the closest existing one: the window was
taken up with another request, nothing was shown for this caller, and
running the command again is the remedy, as for a dialog.
- An outcome this CLI does not know is reported as CLI/app drift (exit
2), like any other unexpected shape, never as success.
The app's own refusals pass through as they already do: `scope_denied`
and `authorization_required` exit 4, and a request the app could not make
(no window left) stays `app_error`.
The verb lives in a new `open` module rather than in `commands.rs`, which
is already over two thousand lines. The mapping takes the `open_project`
call as a closure, so it is tested without an app.
`classify` appends an "update Staged" hint to a 404 on `open_project`, as
it does for the tag commands: an app from before the command answers it
like any unknown command, and no protocol bump tells the two apart.
The project argument is required in this step. Resolving it from the
current directory (step 7) and launching the app (step 8) are separate
commits; the mapping already takes a resolved project, so both can feed
it without reshaping the verb.
A session's credential can list only its own project, so from inside a
session another project's name reads `not_found` from the CLI's lookup,
and the session's own project reads the app's `scope_denied`.
Tested: parsing (required argument, the plural alias, one project only),
the request body, the success output in both forms, each outcome's exit
code and stderr line, the outcome names against the app's, an unknown
outcome, pass-through of the app's refusals, and the 404 hint. The help
fixture is refreshed for the new verb and the `project` noun's summary.
Not run against a live app: `open_project` refuses a session credential,
and this session has no other.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 7 of the `stagedctl project open` plan: directory resolution. Step 6 added the verb with a required project name or id. From a terminal that is already inside a project's worktree, the user still had to look the project up to open it. `stagedctl project open` with no argument now opens the project whose workspace the current directory is in. Staged keeps a project's worktrees under `~/.staged/workspaces/<local|remote>/projects/<project-id>/`, so the id is a segment of the path. In `open.rs`: - `workspace_project_id(path, staged_home)` is the path-to-id mapping. It is purely lexical: it reads nothing from disk or the environment, and takes the Staged home as an input. It matches the project's own directory and anything beneath it, for both locations. A path that climbs with `..` is not matched, since its text does not say where it lands. - `directory_project_id` handles symlinks. The OS reports the current directory by its physical path, while the Staged home comes from `$HOME`, which may reach the same place through a link. When the paths do not match as written, both are canonicalized and compared again. - `target` decides what the verb was pointed at, a name or a directory's id, without the app. A directory outside every project workspace, or a current directory that cannot be read, is `not_in_workspace` (exit 1), in the plan's wording. - `open_target` then confirms the target against the app's project list before `open_project` is sent. An id from a directory is matched on the id alone, never as a name. A directory whose project the app does not have, such as the worktree of a deleted project, is `not_found` and no window is raised. Only an id from the app's list crosses the socket. `commands.rs` settles the target before discovery, as `validate` does for flag rules, so `not_in_workspace` reads the same whether or not Staged is running. Step 8 will launch the app when it is not running, and a directory that names no project must not do that. `list_projects` is split out of `resolve_project` so both kinds of target share one lookup. The project argument becomes optional in `cli.rs`. The help text describes the no-argument form and the help fixture is refreshed. Choices the plan left open: - The older worktree layout, `workspaces/local/<repo>/<branch>/`, is not resolved, per the plan's default. It names no project in its path and reads `not_in_workspace`. - A current directory that cannot be read is `not_in_workspace` rather than an environment error, because the remedy is the same: pass a project. - A workspace directory that is itself a symlink to somewhere outside the Staged home is not resolved. The OS reports the link's target, which carries no id, and `$PWD` is not consulted. Tested in `open.rs`: nested directories, both locations, non-workspace paths, the older layout, `..` paths, a non-UTF-8 segment, a symlinked home, a symlinked `~/.staged`, a directory reached through a symlink, each `target` outcome, and unknown ids. The unknown-id and unknown-name tests panic if `open_project` is called. `cli.rs` covers the optional argument. Run by hand with the built binary: a directory outside any workspace gave `not_in_workspace`; a made-up id under a temporary home gave `not_found` listing the real projects; this worktree resolved to its project and reached `open_project`. The running app predates that command and answered with the update hint, so the open itself was not exercised against a live app. No app-side change, no new backend type and no schema change. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 8 of the `stagedctl project open` plan: launch when not running. Steps 6 and 7 added the verb, by name, id or the current directory, but it needed an app that was already up. With Staged closed it failed with the same "not running under a Staged app" error as every other verb, so the one command a user runs to get to a project could not get there. `stagedctl project open` now starts Staged when no instance is running, waits for the new instance's control server to answer the ping, and then sends the request as before. The app already holds the request until its window's listener is live. `--no-launch` exits 3 instead. The new `launch` module keeps four things apart, so the tests never start the real app: - `launch_target` is the decision. In order: `--no-launch` (exit 3), no credential (exit 4, `authorization_required`), no bundle to start (exit 3). Otherwise it names the bundle. - `bundle_for_executable` reads the bundle off the CLI's own path: `<bundle>.app/Contents/MacOS/<exe>`. It is purely lexical. `LaunchEnv::real` resolves symlinks first, because `/usr/local/bin/stagedctl` is a link into the bundle. - `wait_for_instance` polls for a discovery file that was not there before the launch and passes the handshake, for up to 30 seconds. - `Launcher` is the one spawn. The real one runs `/usr/bin/open <bundle>` and nothing else, on macOS only. `connect_or_launch` joins them, and `open::run` is now the verb from the top: target first, then find or start Staged, then open. What may be started, per the plan's "Security" and "Behaviour by state": - Only the bundle this executable is in. Nothing from the arguments or the environment names it, and `open` is run by absolute path. - Never for a caller with no credential. A grant can only be proven to a running app, so the precheck is that an unexpired grant file exists on this machine. Per the plan's default, the consent dialog is not triggered; the message points at `stagedctl connect`. - Never a development build, whose executable is not in a bundle and whose app needs its dev server. The message says to run `just dev`. - Never for a directory outside every project workspace. Step 7's order is kept: `open::target` runs before the app is looked for, so `not_in_workspace` reads the same with or without `--no-launch`. Choices the plan left open: - A named control file never starts an app. `--lock-path` and `$STAGEDCTL_LOCK` point at one instance, and a fresh app cannot make that file valid. So a spawned session never launches one. - "Not running" needs to be told from "running but unusable", such as an app that speaks another protocol version. Starting a second Staged beside that one is not the remedy. `CtlError` gains `NotRunning` for the three failures that mean no app is there: no live instance directory, a control file that is gone, and an endpoint nothing answers. It exits 3 and prints exactly as `Environment` did, so no other verb's output changes. Staged is started only when every candidate failed that way, which also covers a crashed instance's leftover directory. - The wait skips the candidates that were on disk before the launch, by path and mtime, so a leftover directory costs nothing per poll and a rewritten file at the same path still counts as new. A new instance that is not answering yet is retried; one that answers but cannot be used fails at once. - The precheck does not read a grant's access or scope. Which grant the app accepts is decided at reconnect, so a prediction here could disagree with it. A read-only grant therefore still starts Staged and then reads the app's `scope_denied`. - A name is looked up in the app's list, so a name that matches nothing is `not_found` only after Staged has started. - Instance choice stays newest-first and the pid is still printed. A line on stderr says Staged is being started; stdout is unchanged. `client.rs` also gains `live_discovery_candidates`, the existing live-pid listing with each file's mtime, and its test module's fake servers and fixtures become `pub(crate)` for the launch tests. The help text for `project open` describes the launch and each case in which nothing is started, and the help fixture is refreshed. The guide and the `staged` launcher script are step 9. Tested in `launch.rs`: bundle paths (installed, renamed, nested, development builds, unresolved links, relative and `..` paths, a real symlink), the `open` invocation without running it, each decision and its order, the precheck, which candidates count as running, the wait (a file that appears later, stale entries never probed, a rewritten file, a retry, a fast failure, a timeout), and `connect_or_launch` with a running app, none, a leftover directory, `--no-launch`, no grant, a development build, an unusable app, a named control file, a failed launch and a launch that never answers. `open.rs` covers `run`: a start followed by the two app calls, a workspace directory, an unknown name, `--no-launch`, and a directory outside every workspace with a launcher that panics if reached. `client.rs` pins which failures are `NotRunning`, and `cli.rs` the flag. Run by hand with the debug binary under a temporary home, with `$STAGEDCTL_LOCK` unset: no grant gave `authorization_required` (exit 4), `--no-launch` gave exit 3, a grant file gave the `just dev` refusal (exit 3), a missing `--lock-path` gave the usual control-file error, and a directory outside any workspace gave `not_in_workspace`. Not run against a real app. Every successful launch above is against a fake launcher and a fake control server: the real `open` call, the wait on a real cold start, and the request reaching a window that is still loading were not exercised. This session runs inside Staged, so the app was not started, quit or restarted. CLI only: no app-side change, no schema change and no backend type. The new types (`Launcher`, `LaunchEnv`, `DiscoveryCandidate`, the `NotRunning` variant) are in the stagedctl crate. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 9 of the `stagedctl project open` plan: docs and launcher. Steps 1 to 8 built the command, but nothing outside its own help described it, and the two places a user would look were wrong. The README said `staged [path]` opens a directory and that each run opens a new window. The app never read that argument, and the script started a second Staged process on every run. `scripts/staged` is now a thin wrapper: - Every argument goes unchanged to `stagedctl project open`, by `exec`, so the output and exit codes are stagedctl's. - `stagedctl` is taken from PATH, then from `/Applications/Staged.app` and `~/Applications/Staged.app`. A release DMG install puts nothing on PATH today, so the fallback keeps `staged` working there. - The script never starts the app itself. When `stagedctl` is not found it says so on stderr, prints nothing on stdout and exits 127. - The lookup uses `type -P`, so an exported shell function named `stagedctl` is not mistaken for the binary. The README's "Command Line Usage" section is rewritten from `open.rs`, `launch.rs` and the verb's help text rather than from the plan: - opening by name or id, and with no argument from inside a project workspace; - `--no-launch`, what happens when Staged is not running, and each case in which it is never started; - the first-run `stagedctl connect`, and that read-only grants and agent session credentials are refused; - the `staged` shorthand, and a table of exit codes and error codes; - how `stagedctl` gets on PATH today: the source install links it, and a release DMG install needs a manual link into the bundle. The claims that `staged` opens a path and that each run opens a new window are gone. The install bullet now names both CLIs. `guide.md` gains a "Showing a project in the app" section with the same facts, written for the agent sessions that read it. It leads with the refusal: a spawned session cannot use the verb, even for its own project, and should name the project for the user instead. The safety invariant section says how the verb fits it. `install.sh` printed "Run `staged` to launch Staged from your current directory" when it finished. It now prints the connect step and the two forms of `staged`. Choices the plan left open: - `staged` takes no directory argument. The old usage line offered one; a path is now read as a project name and fails with `not_found`. Mapping a path to a directory change was left out to keep the wrapper thin: `cd <dir> && staged` does the same. - 127 for a missing `stagedctl` is the shell's "command not found", and sits outside stagedctl's own 0 to 4. - The old lookup of a bundle built inside the repository is dropped. A build there is reached by putting its `stagedctl` on PATH. - The Settings action that will put `stagedctl` on PATH is step 10 and is not mentioned. No test pins the guide, the README or the launcher, and the help text is unchanged, so no fixture moved. No Rust or frontend source changed. Checked: - `just fmt-check` passed. It covers Rust and `src/`, which this commit does not touch. - `bash -n` passed on `scripts/staged` and `install.sh`. shellcheck is not configured in the repository and is not installed here, so it was not run. - The launcher was run by hand against a fake `stagedctl` that echoes its arguments: no argument, a name with spaces, flags either side of the name, `--`, empty and quoted arguments, `--help`, exit codes 1 to 4 passed through, a symlinked script, `/bin/bash` 3.2, the bundle fallback, PATH taking precedence over the bundle, a non-executable bundle file, an exported function, and `stagedctl` missing with and without `$HOME`. - It was also run over this branch's debug `stagedctl` under a temporary home with `$STAGEDCTL_LOCK` unset: `--help` printed the verb's help, `--no-launch` exited 3, a directory outside any workspace exited 1 with `not_in_workspace`, no grant exited 4 with `authorization_required`, and two projects exited 1 as a usage error. Not run against a real app. No project was opened through the wrapper, and the running Staged was not started, quit or restarted; its process was the same before and after. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Step 10 of the `stagedctl project open` plan, and its last: PATH for release installs. Steps 1 to 9 built the command and documented it, but a release DMG install puts nothing on PATH. The CLI sits inside the bundle, only agent sessions get a shim for it, and the README told everyone else to make a symlink by hand. Settings -> Connected apps now has a "Command line" section with an "Add to PATH" button. It calls one new Tauri command, `link_stagedctl_on_path`, which links the app's bundled sidecar to `/usr/local/bin/stagedctl`, or to `~/.local/bin/stagedctl` when `/usr/local/bin` cannot be written. It never asks for a password. The backend is a new `stagedctl_link` module in three parts, so that no test writes outside a temporary directory: - `stagedctl_link.rs` holds the decisions and touches no filesystem. `link_source` reads the link's target off the app executable's path, or refuses. `link_candidates` lists where the link may go, in order. `decide` picks one step from what is already at each path. `dir_on_path` says whether the chosen directory is on PATH. - `stagedctl_link/fs.rs` looks at the disk, runs the step `decide` chose and looks again if the disk changed underneath. Every path is handed to it. - `stagedctl_link/commands.rs` is the command. It is the only place that reads the real executable, home directory and PATH. What the action does, following the plan's "Getting onto PATH" and "Security" sections: - The link's target is this app's own sidecar, `<bundle>.app/Contents/MacOS/stagedctl`. The command takes no path, and nothing from the frontend or the environment names the target. - It refuses when the app runs from an App Translocation path. macOS runs a quarantined app from a randomized read-only mount, and a link into it would dangle. The message says to move Staged to Applications and reopen it. - It refuses a development build, which has no bundle and whose sidecar goes when the build directory does. - It never overwrites what it has not shown. A path that already holds something else comes back as a `conflict` outcome and is left alone. A link that points elsewhere is shown with its target and a "Replace link" button. That press sends back the link and target that were on screen, and the link is replaced only if it is still that one. A file or directory is never replaced. - A link that already reaches the sidecar, however its target is spelled, is reported as already linked and not rewritten. - The fallback is only for a directory that cannot be made or written. A conflict in `/usr/local/bin` is never skipped for `~/.local/bin`: the earlier `stagedctl` could shadow the new link, and the user should see it. - After linking, the answer says whether the directory is on the user's PATH, read from the cached login-shell environment. The panel names the directory to add when it is not, and says so when PATH could not be read. Who may call it: - Registered in `generate_handler!` beside the other stagedctl commands. - Refused by the web dispatch, with the other desktop-only arms. A browser client must not change what the user's shell runs. - Left out of `ALLOWED_COMMANDS`, so no stagedctl caller, an agent session included, can reach it. Choices the plan left open: - The action lives in the Connected apps panel, which is already desktop-only and about stagedctl, as a small `StagedctlPathSetting` component. - One command, with no separate status query. The panel shows a result only after a press. Pressing again is safe: a link that is already right is reported, not rewritten. - A replace swaps the link with a rename, so the name on PATH is never missing. The check and the rename are two calls, the closest the filesystem allows to a compare-and-swap. - A replace that the directory refuses is an error, not a reason to link elsewhere. - An app run straight from a mounted disk image that macOS did not translocate is not refused. Its path looks like any external volume. Docs: - `README.md` points release installs at the Settings action and keeps the manual link as the "By hand" fallback. - `guide.md` did not describe a manual link before. It now says how `stagedctl` gets on PATH outside a session, that the Settings action is the user's and not a verb, and gives the manual link as the fallback. - The `staged` launcher's "stagedctl was not found" message names the Settings action ahead of the manual link. This adds backend types, which AGENTS.md asks a human to review: `LinkOutcome`, `ExistingEntry` and `ShownLink` cross to the frontend, and `Refusal`, `Entry`, `Candidate`, `Step` and `Installed` are internal. Nothing is persisted and there is no schema change. Tested: - `stagedctl_link.rs`, 23 tests: the link target for several install locations, the translocation refusal and its order against the bundle check, a path that only mentions the word, executables outside a bundle, the candidate order, every branch of `decide`, the PATH check and the wire shape. - `stagedctl_link/fs.rs`, 16 tests in a temporary directory: create, second run, a link spelled another way, both fallbacks, no usable directory, a link elsewhere, a dangling link, replace, a link that changed after it was shown, a file, a directory, a replace the directory refuses, a missing sidecar and a link already in the fallback. - `StagedctlPathSetting.component.test.ts`, 11 tests: each outcome's copy, the PATH hints, replace only on request and with what was shown, no replace for a file, errors, and presses while a request is in flight. `ConnectedAppsPanel.component.test.ts` gains one test. Checked: - `cargo fmt --check` and `prettier --check` on `src/` passed. - `cargo clippy --workspace -- -D warnings` passed. - `cargo test --lib -- stagedctl_link web_server control_server` passed, 165 tests. That includes `web_dispatch_covers_tauri_commands`, the registration and web-mode parity check, and the pinned allowlist tests. - `pnpm -s check` passed with no errors. - `vitest run` on the two component test files passed, 17 tests. - `bash -n scripts/staged` passed. - The panel was rendered in a Vite harness under /tmp with the command's answers faked, and each state was screenshotted at two widths. Not exercised in a real app. The command was never invoked through Tauri, so the real executable path, the real `/usr/local/bin` and `~/.local/bin`, the login-shell PATH lookup and the translocation refusal on a translocated app are untested outside unit tests. No link was created or changed in a real PATH directory, and the running Staged was not started, quit or restarted. The full Rust and frontend suites were not run. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Two warnings from the review of `stagedctl project open`, both in how `open_project` chooses and raises a window. Both were checked against the code first and hold. Which window. `pick_window_to_show` took the focused window, then "any visible" one from `webview_windows()`. That map is built fresh on every call, so with two or more visible windows and none focused the pick could differ between two runs. That is the normal state for this command: the terminal is frontmost, so no Staged window holds focus. The help text promised "the window a click on Staged's Dock icon would raise". Cold start. Every window is created hidden (`visible: false`, and the window-state plugin leaves visibility out) and its frontend calls `show()` at the end of mount, after the theme is applied. `open_project` arrives as soon as the control server answers, and `raise_window` showed the window itself, bringing back the unpainted flash the hidden start is there to avoid. The pick now goes: focused, then the window the user last used, then a visible one, then one that needs unhiding. The last two tiers go in a fixed order: `main`, then the `win-N` peers by number. - A new `app_lifecycle::window_use` module holds `UsedWindows`, an in-memory list of window labels in the order they were last used. - A window goes on it when it gains focus (`WindowEvent::Focused(true)`), when the last window is closed to hide, and when the backend raises it. It comes off when it is destroyed. - When the last-used window is gone, the one used before it is tried. The raise for `open_project` is now `raise_window_once_revealed`: - A window its frontend has revealed is raised on the spot, as before. That is one on screen (visible or minimized), or one on the list, which covers a window hidden since, such as the last window closed to hide. - Any other window is still loading and is left alone. The request is still parked for it and it still answers from its snapshot. The raise follows once the window comes on screen, so the frontend's own reveal is the first and the window still ends up in front and focused. - Nothing reports a window going on screen, so a spawned task polls for it every 100 ms. It stops when the window is destroyed or after 60 seconds. The reveal can come after the answer, so the task outlives the call. Choices worth a look: - The backend is not told when a frontend calls `show()`, so "revealed" is inferred: on screen now, or used before. Telling it would need a new command, or a window permission the frontend does not have. - On macOS the frontend's `show()` is `makeKeyAndOrderFront`, which already fronts the window when Staged is the active app. The deferred raise is for when it is not: the user went back to the terminal while Staged loaded, or Staged was started some other way and is still loading. - The Dock reopen and `Window > Staged` still raise unconditionally. Their usual cases pick as before: one window closed to hide, or a focused window. One case changes: several windows, none focused and none visible (all minimized, or the app hidden). That used to land on `main` and now lands on the window used last. - Where it was arbitrary before, among several visible or several hidden windows that were never used, it is now the first in the fixed order. The help text, README, guide and two comments now say the window is the one the user was last in, and that a window still loading comes to the front once it is ready, which can be after the command has exited. The help fixture is refreshed. This adds a backend type, which AGENTS.md asks a human to review: `UsedWindows`, managed app state. Nothing is persisted and there is no schema change. Tested on the mock runtime: - `window_raise.rs`: each tier of the pick, the last-used window with several visible, the fallback when it is gone, the fixed order run repeatedly, the list read through the app-level pick, a raise recorded on the list, an immediate raise for a window on screen and for a used window that is hidden, a loading window raised only once shown, and a window never shown given up on. - `control.rs`: the last-used window told and no other across repeated calls, `main` told when none was used, a window on screen raised before it answers, a loading window parked, answered and raised only after its reveal, and a loading window whose caller timed out still raised. - `window_use.rs`: order, repeat, forget and an unknown label. Checked: - `cargo fmt --check` passed. - `cargo clippy --workspace -- -D warnings` passed. - `cargo test --lib -- app_lifecycle project_open window_commands web_server control_server` passed, 199 tests. - `cargo test -p stagedctl` passed, 175 unit tests and the help fixture. - `pnpm -s check` passed with no errors. - `vitest run` on `projectOpenListener.test.ts` and `navigation.test.ts` passed, 86 tests. The only frontend change is a comment. Not exercised in a real app, and the full Rust and frontend suites were not run. The mock runtime reports every window as visible, sends no focus events and never removes a destroyed window, so these are untested: - the focus event reaching the list, and the list being cleared on destroy; - `is_visible` and `is_minimized` on a real hidden, minimized or app-hidden window; - the cold start end to end: no flash, and the window fronted after its reveal; - the poll stopping early for a destroyed window. The running Staged was not started, quit or restarted. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Two warnings from the review of `stagedctl project open`, both in what the window does with a request. Both were checked against the code first and hold. The two window-pick and cold-start-raise warnings were fixed in 1637a00; the review's suggestions are left for a separate commit. Cold start. The window refuses a request with `dialogOpen` while a dialog is open. On a cold start two chains of round trips ran side by side in the main window: - the listener: register, read the snapshot, wait for the project list, then look for a dialog; - the mount: restore unread state, load the key bindings, read the store status, then `resumeOnLaunch.load()`, which opens the "Resume interrupted sessions?" dialog when the last quit interrupted sessions. Nothing ordered them, so the same command could end `opened` with the dialog over the new project, or `ui_busy` telling the user to close a dialog in an app they had only just started. A request found at startup is now acted on before the offer is loaded: - `listenForProjectOpens` returns `{ stop, startupHandled }` instead of the bare unlisten function. `startupHandled` settles once the listener is live, the snapshot has been read, and every request taken by then has been opened and answered. - `App.svelte` chains `resumeOnLaunch.load()` on it, in the place it was called before. The project opens and the offer then appears over it. - With nothing parked the wait is two round trips that start well before the offer's turn, so the offer is not held up in practice. - The offer cannot be lost. `startupHandled` also settles when the registration or the snapshot fails, when the listener is stopped, and after 15 seconds at the latest. That mirrors the backend's `ANSWER_TIMEOUT`: by then the caller is being told it timed out whatever a dialog does. This is the ordering option rather than exempting the dialog. It is safe here: the offer is mounted at the app root, so no project view owns it, and the backend holds the interrupted set until it is answered, so loading it later loses nothing. It covers the request the window finds waiting, which is the cold start the review describes: the CLI sends the request as soon as the control server answers, well before the frontend runs. A request that reaches the window after its startup snapshot, while the offer is already open, is still refused like any other request with a dialog in the way. Stale navigation. A request the listener had taken could wait on the project list, on a diff comment being saved, or behind an earlier request. If the backend's wait ran out, or a newer request superseded it, the caller was told so, but `selectProject` still ran and only the answer was refused. The window now checks that the request is still pending right before it navigates: - `openProjectExternally` takes an optional `stillWanted` check. It is awaited last, after the wait for the project list and after a diff save. When it resolves false nothing is navigated and the result is `withdrawn`. - The project and the dialogs are looked at again after the check, in the same tick as the navigation, because the check is a round trip. - The listener passes a check that reads `list_pending_project_opens` and matches on the request id. A `withdrawn` result sends no answer. - A check that fails is treated as not wanted: the helper rejects, nothing is navigated, and the caller reads a timeout. - The check is not made when nothing would be navigated: a project that is already on screen, and each refusal. One gap is left and is now documented rather than denied. The timeout or the newer request can land between the snapshot's reply and the navigation it allows, one round trip to the window. The window then switches and its answer is refused. Closing it would need the backend to hold its lock across the window's navigation. Wording brought in line with that: - The module doc in `project_open.rs` no longer says a window "never navigates for a caller that has already been told it failed". It says what the state does, what the window's check adds, and what gap is left, and that `TimedOut` and `Superseded` mean the window did not report showing the project. - The CLI's superseded message said the project "was not shown". It now says the request was replaced "before the window answered it" and that the project "may not be showing". - The verb's help text and the README's `ui_busy` row say "before the window answered it" in place of "before the window acted on it", and no longer tie "not switched" to a superseded request. The help fixture is refreshed. No new backend type, command or state, and no schema change. The Rust changes are doc comments and one message string. Tested: - `projectOpenListener.test.ts`, 16 new tests. For stale requests: the check asks the backend afresh and matches on the request id, a `withdrawn` result is not answered, a request withdrawn while the helper waits is not navigated, one still pending is, one superseded while the helper waits gives way to the newer one, one withdrawn while it waited its turn is not navigated, and a failed check navigates nowhere. For startup: `startupHandled` settles only after the parked request is opened and answered, after the snapshot when nothing was parked, after a request the event delivered first, after one that arrives during the parked one, and on a withdrawn request, a failed snapshot, a failed registration, a stop, and the 15 second cap. - `navigation.test.ts`, 7 new tests on the real helper: a request withdrawn during the wait for the list, and during a diff save, is not navigated; the check runs after the save; it is skipped when nothing would be navigated; the dialogs and the project are read again after it; and a check that rejects navigates nowhere. - Each fix was removed in turn and the new tests failed: the listener passing no check, `startupHandled` settling before the queue drained, the helper ignoring the check, and the helper asking before its waits. Checked: - `prettier --check` on `src/` passed. - `pnpm -s check` passed with no errors or warnings. - `vitest run` on `projectOpenListener.test.ts` (29 tests) and `navigation.test.ts` (80 tests) passed, as did the other listener tests and `transport.test.ts` alongside them, 227 in all. - `cargo fmt --check` passed. - `cargo test -p stagedctl` passed: 175 unit tests and the help fixture. - `cargo test --lib -- project_open` in the app crate passed, 22 tests. - `cargo clippy --workspace -- -D warnings` passed. Not exercised in a real app, and the full Rust and frontend suites were not run. Unverified without a live app: - `App.svelte`'s chaining of the offer on `startupHandled`. No unit test mounts `App.svelte`; the tests cover the promise it waits on. - The cold start end to end: a request parked before the window loads, with sessions interrupted by the last quit, ending `opened` every time with the offer over the project. - That the request really is parked before the window's startup snapshot on a real cold start, which the ordering relies on. - The check against the real backend: a caller timing out, or being superseded, while a real window waits on its list or on a diff save. The running Staged was not started, quit or restarted. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The four suggestion-level points left from the review of `stagedctl project open`. Its four warnings were fixed in 1637a00 and 128c4bb and are not touched here. Each point was checked against the code first, and all four hold. Point 3 holds in a narrower form than the review's wording, as described below. 1. A Staged that is alive but slow is picked up by the launch wait (`launch.rs`). `Client::connect` reports any failed ping as `NotRunning`, a timeout included. So a Staged too busy to answer was taken for absent, and the bundle was opened, which for a running app only brings it forward. The wait then skipped that instance for the full 30 seconds, because its discovery file had the same path and mtime as before. The error said Staged "was started". - `wait_for_instance` now also tries the instances that were on disk before the launch. They are tried after the new ones, and only on every tenth poll (`KNOWN_PROBE_EVERY`, about every two seconds). Most are a crashed instance's leftover, and those must not slow the wait for a new instance. - One that now answers but cannot be used, such as another protocol version, does not end the wait: a new instance may still be coming. Its error is reported in place of the timeout message if nothing else answers. - The timeout message is now "stagedctl asked macOS to open Staged, but no Staged control server answered within 30 seconds". It no longer claims an app was started. - The stderr line before the open had the same flaw and is now "no running Staged answered; opening <bundle>". - The deadline is checked between polls, so the wait can run over by one probe of an instance that accepts a connection and says nothing. 2. A workspace outside a confined credential is not called deleted (`open.rs`). With a grant approved for one project, `list_projects` returns that project alone. Running `stagedctl project open` from another project's worktree then said Staged "does not have" the project and that it "was probably deleted". - `open_target` takes the client's `CredentialScope` and passes it to `project_for_directory`. - For `CredentialScope::Project` the message says the credential does not reach the project, names the one project it is confined to, says the project may still exist, and points at `stagedctl connect --help`. - The code stays `not_found` and the exit code stays 1. The unconfined wording is unchanged. 3. A leftover staging link no longer blocks a replace (`stagedctl_link/fs.rs`). `replace_link` makes the new link at `.stagedctl.link-<pid>` and renames it over the old one. The name carries the app's process id, so a leftover from another process has another name and was never in the way. A leftover at this process's own name does block: every later replace fails with "File exists". That needs a replace cut short after the link was made or a clean-up that failed, in this process or in an earlier one that had the same id. - `clear_stale_staging` runs before the staging link is made. It removes what is at exactly the staging name, and only if that is a symlink. - A file or a directory there is left as it is. The replace is refused with a message naming the path, in place of a bare "File exists". - Staging names carrying another process id are not looked at, and a plain create never touches the staging name. 4. `staged <directory>` opens from that directory (`scripts/staged`). `staged .` and `staged /path/to/repo` were the documented usage before this branch, and had become a project name that failed with `not_found`. - When the one argument is an existing directory, the script changes to it and runs `stagedctl project open` with no project, so the project is resolved from the directory. - `--no-launch` and `--json` may sit beside the directory. Any other flag, a second argument, or `--` leaves every argument unchanged. - A name that is not an existing directory is passed on exactly as before. - A relative directory is entered as `./<dir>`, so `$CDPATH` is not searched. A `stagedctl` found through a relative PATH entry is made absolute first. - A directory that cannot be entered exits 1 with a message. Choices worth a look: - An existing directory always wins over a project of the same name. The README says to use `stagedctl project open <name>` for that case. - The script knows two of stagedctl's flags by name. Without that, `staged . --no-launch` would read `.` as a project name. - A directory outside every project workspace still fails with `not_in_workspace`, without starting Staged. `staged` does not open arbitrary repositories. Docs: the README's launcher section, exit code table and "When Staged is not running" bullet, the guide, and the verb's help text describe the directory form, the confined-credential wording and the slow instance. The help fixture is refreshed. Tests: - `launch.rs`, 5 new and 3 updated: a known instance used once it answers, the probe cadence for a leftover, new instances first and on every poll, an unusable known instance not ending the wait, its error reported at timeout, the message text, and `connect_or_launch` with an instance that starts answering only after the open. The fake app for that one binds a port its discovery file already names, so `client.rs`'s test helpers gain `grant_aware_server_on`. - `open.rs`, 2 new: the confined wording with and without a listed project, and a confined credential still opening its own project. - `stagedctl_link/fs.rs`, 4 new, all in a temporary directory: a leftover link cleared whether or not it dangles, a file and a directory at the staging name left intact and reported, another process's staging link left alone, and a create not touching the name. - Each fix was disabled in turn and the tests for it failed: 6 in `launch.rs`, 1 in `open.rs`, 2 in `fs.rs`. Checked: - `cargo fmt --check` passed. - `cargo clippy --workspace -- -D warnings` passed. - `cargo test -p stagedctl` passed: 182 unit tests and the help fixture. - `cargo test --lib -- stagedctl_link` in the app crate passed, 43 tests. - `bash -n scripts/staged` passed under `/bin/bash` 3.2, the only bash on this machine. - The launcher was run by hand against a fake `stagedctl` that echoes its arguments and working directory: no argument, names, flags, `.`, relative, nested and absolute directories, a trailing slash, a symlink to a directory, both flags around a directory, a name that is also a directory, two arguments, an unknown flag, `--`, `--help`, an empty argument, a directory named with a leading dash, a regular file, `$CDPATH`, a directory that cannot be entered, a relative PATH entry, the bundle fallback, `stagedctl` missing, a symlinked script, and exit codes 1 to 4 passed through. - It was also run over this branch's debug `stagedctl` under a temporary home with `$STAGEDCTL_LOCK` unset: a directory outside any workspace exited 1 with `not_in_workspace`, a made-up workspace directory with `--no-launch` exited 3, the same without a grant exited 4, and a name with `--no-launch` exited 3. Not exercised in a real app, and the full Rust and frontend suites were not run. Unverified without a live app: - A real Staged that is slow to answer its ping, and `open` bringing it forward instead of starting a second one. - The confined-credential message against a real one-project grant. - A replace through the Settings action with a leftover in a real PATH directory. No link was created or changed outside a temporary directory. - `staged <directory>` opening a project in a running app. The running Staged was not started, quit or restarted. No backend type, command or state is added, and there is no schema change. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The three open points from the second review of `stagedctl project open` (#review:2a5dad41-bd21-49bc-b883-8d8ce61e7fab): one warning and two suggestions. Its other comments are confirmations. Each point was checked against the code first, and all three hold. 1. A revealed window that never held focus is raised (`window_reveal.rs`). 1637a00 stopped `open_project` showing a window before its frontend had. The backend was not told when a frontend called `show()`, so "revealed" was inferred: on screen now, or on the record of used windows, which a window joins when it gains focus, is closed to hide, or is raised. A window can be revealed and fit neither. Staged started in the background, or launched hidden, shows its window without it taking focus. Hide that app from another one and the window is no longer on screen either. `open_project` then took it for a window still loading: the request was parked, the live frontend answered `opened`, the CLI exited 0, and Staged never came forward. A 100 ms poll waited 60 seconds for a reveal that had already happened, then logged and gave up. The reveal is now reported instead of inferred: - The frontend calls a new Tauri command, `note_window_revealed`, right after its own `show()` at the end of mount, once the theme is applied. `revealWindow()` in `services/windowReveal.ts` is the two together, and `App.svelte` calls it where it called `show()`. - A new `app_lifecycle::window_reveal` module holds `RevealedWindows`, managed app state: the windows that have been revealed, and the windows owed a raise once they are. - `raise_window_once_revealed` reads that record and nothing else. A revealed window is raised on the spot, whatever it looks like now. Any other window is still loading: it is left alone and marked as owed a raise. - The owed raise is made by the command, when the reveal is reported. The frontend's own reveal is still the first, and the window still ends up in front and focused. - The poll and its spawned task are gone. The 60 second bound stays as a deadline on the owed raise: a reveal reported later than that is recorded but raises nothing. - A window comes off the record when it is destroyed, with any raise it was owed. The cold start from 1637a00 behaves as before. A window its frontend has not revealed is not shown early, the request is still parked and answered from the snapshot, and the raise follows the reveal, now at once instead of within 100 ms. Who may call the command: - Registered in `generate_handler!` beside the other window commands. - Refused by the web dispatch: a browser client has no native window. - Left out of `ALLOWED_COMMANDS`, so no stagedctl caller can have a loading window shown early. - It takes no arguments and acts on the calling window only. Choices worth a look: - A window the backend shows itself also goes on the record. The Dock reopen and `Window > Staged` raise without waiting for the frontend, so after one there is no first paint left to protect. That raise also settles any raise the window was owed, so the frontend's later reveal does not pull it forward a second time. - `hide_window` no longer puts a closed window on the record of used windows. That line existed only to feed the inference. Only the last window is closed to hide, so the pick is unchanged. - The record of used windows now only orders the pick. Its `contains` is test-only. - `open_project_within` loses its on-screen probe and is back to the shape it had before 1637a00. `is_on_screen` is removed. - With no record managed, as on most mock apps, the window is raised at once. The real app always manages one. - A report that fails is logged and dropped by the frontend, and the mount carries on. The window would then read as still loading until the backend next raises it itself. 2. A session is not told to use a grant (`open.rs`). `project open` run from a workspace the credential does not reach said to "run the command with a grant that covers it (see `stagedctl connect --help`)". The same branch is reached by an agent session's lease, and the app refuses `open_project` to every session, for its own project too. The remedy sent a session after a credential that does not exist. The client already knows which it holds: a per-session control file means a lease, and a grant file says what it was approved for. `CredentialScope` merged the two as `Project`. It now has a third variant, `Session`, set where the client sees a per-session control file. - A one-project grant keeps its wording, word for word. - A session reads that its credential is confined to the session's own project, that the project may still exist, that no credential lets a session open it, and to name the project for the user. It mentions neither a grant nor `stagedctl connect`. - The code stays `not_found` and the exit code stays 1. - `tags.rs` treats the new variant exactly as `Project`: `usageScope` is still `"project"` and the scope note still prints. 3. A bare word is always a project name (`scripts/staged`). An existing directory always beat a project name. Projects are usually named after their repository, so `staged builderbot` run beside a `builderbot/` folder changed into it and failed with `not_in_workspace`. - The directory form is taken only when the one argument is written as a path, `.`, `..` or anything with a slash in it, and is an existing directory. - A bare word goes to `stagedctl project open` as a project name, whether or not a directory of that name exists. - `staged ./builderbot` and `staged builderbot/` mean the folder. - An argument with a slash that is not an existing directory is passed on unchanged, as before. A project name with a slash in it that is also an existing directory still needs `stagedctl project open <name>`. - The "cannot change to the directory" message now shows the argument as it was typed. It used to show the `./` the script adds for `$CDPATH`. Docs: the README's launcher section lists the path rule, the bare-word rule and both forms in its examples. The guide's launcher bullet says the same, and its session paragraph now covers a session run from another project's workspace. The script's header is rewritten. The verb's help text does not describe the launcher, so the help fixture is unchanged. This adds a backend type, which AGENTS.md asks a human to review: `RevealedWindows`, managed app state, and one Tauri command. Nothing is persisted and there is no schema change. `CredentialScope::Session` is a new variant in the stagedctl crate. Tests: - `window_reveal.rs`, 18 new. The record: a reveal with nothing owed, one in time, one too late, a newer request extending the wait, a revealed window owed nothing, and a forgotten window. On the mock runtime: a revealed window raised at once, one that was never used raised at once, a loading window raised only when its frontend reports, a used window that is not revealed still waited on, the deadline on either side, a reveal raising only its own window, a repeated reveal, a window the backend showed, a destroyed window, and an app with no record. The mock runtime calls every window visible, so these also show that visibility is not consulted. - `window_raise.rs`: the 5 tests of the poll and the probe are removed with them. Their cases are covered in `window_reveal.rs`. - `control.rs`, 3 new and 3 reworked: a revealed window that never held focus is raised as it is asked, a reveal while the caller waits raises before the answer, another window's reveal does not raise the loading one, and the cold-start and timed-out cases now reveal through the command. - `windowReveal.test.ts`, 6 new: the report follows the show, waits for it, is skipped when the show fails, a failed report still resolves, and web mode reports nothing. - `open.rs`, 2 new: the session wording in full, with and without a listed project and with no mention of a grant, and the grant wording kept. `client.rs` and `tags.rs` each have one test updated for the new variant. Run in full, after the last edit: - `cargo fmt --check` passed. - `prettier --check` on `src/` passed. - `cargo clippy --workspace -- -D warnings` passed. - `cargo test --workspace --lib --bins --tests` passed. App crate: 2200 passed, 0 failed, 2 ignored. stagedctl: 184 passed, 0 failed. Help fixture: 1 passed. The 2 ignored are the live `project_mcp` tests that spawn the installed agent bridge and call the model. - `pnpm run check` passed: 0 errors and 0 warnings. - `vitest run` passed: 239 files, 3598 tests, 0 failed. - `bash -n scripts/staged` passed. Nothing failed, so no comparison with the commit before the branch was needed. The launcher was run by hand under `/bin/bash` 3.2 against a fake `stagedctl` that echoes its arguments and working directory: - bare words: a name that is also a directory, a symlink to a directory, a name with a space, a missing name, a regular file, and each beside `--no-launch` or `--json`; - path forms: `.`, `..`, `./dir`, `dir/`, nested, absolute, a symlink, a directory with a space, `./-dash`, a directory named `--json`, and a slash name with and without a directory behind it; - left unchanged: two arguments, an unknown flag, `--`, `--help`, an empty argument and a bare `-dash`; - `$CDPATH` set, for a bare word and for `./word` that exists only under `$CDPATH`; - a directory that cannot be entered, a relative PATH entry, exit codes 1 to 4 passed through in both forms, `stagedctl` missing, and a symlinked script. Not exercised in a real app. The mock runtime sends no focus or destroy events and reports every window as visible, so these are untested: - the command invoked through Tauri from a real window, and the raise it makes from that call; - the original failure end to end: Staged started in the background, then hidden, then `project open` bringing it forward; - the cold start end to end: no flash, and the window fronted as its frontend reveals it; - the record being cleared when a real window is destroyed; - the session wording against a real session lease. The running Staged was not started, quit or restarted. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5ae89ad57
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| async function loadPending(): Promise<void> { | ||
| try { | ||
| for (const request of await listPendingProjectOpens()) take(request); |
There was a problem hiding this comment.
If the startup snapshot captures request A but returns after events for B and C have arrived, with B still being processed and C waiting, passing A to take() unconditionally replaces C in the single waiting slot. Once B finishes waiting, both B and A fail stillPending() because the backend holds C, but C is never processed or answered. The latest caller therefore gets ui_timeout and its project never opens. This ordering is reproducible against the listener with deferred snapshot and navigation promises. Keep a delayed snapshot from overwriting requests already queued by newer live events.
Useful? React with 👍 / 👎.
matt2e
force-pushed
the
cli-command-to-open-project
branch
from
November 14, 2026 01:12
f5ae89a to
a450322
Compare
matt2e
added a commit
that referenced
this pull request
Nov 14, 2026
…commands CI on PR #1069 failed one test at e872926, deterministically: `web_server::blocking_guard_tests::synchronous_commands_touch_only_in_process_state` rejected `rename_project`. Main added that command in 6a8fe91 (#1044) as a plain `fn`, after this branch's Fix A commits, so it ran on the main thread and froze the window for its store write, its read and any wait on the store lock. The guard is right, so the command is converted, not allowlisted. The branch was first rebased onto origin/main 485b576 (#1071), with no conflicts; the nine commits are unchanged by range-diff. On that tree the four guard tests were run before the fix: the one above fails, for `rename_project` only, and the other three pass. Main then moved again while CI ran the first push of this fix (c036996). 22c8335 (#1072, `stagedctl project open`) added three plain `fn` commands, and the same guard rejected them as unlisted. The branch is rebased onto 22c8335, again with no conflicts, and they are handled below. Converted - `rename_project` is now an `async fn` that only looks up the store handle and hands its whole body, `rename_project_impl`, to `tauri::async_runtime::spawn_blocking`, the pattern of d99d9f7. The body is unchanged. - It is added to `OFFLOADED_COMMANDS`, so the command guard and the arm guard hold it to the handoff-only rule. - Its web `dispatch` arm called `rename_project_impl` inline on a runtime worker. It now goes through `blocking_json`, with the same result and the same error text. Listed as synchronous The guard's scan found nothing that blocks in #1072's three commands, but it does not follow method calls, so each was read before it went into `SYNCHRONOUS_COMMANDS`: - `list_pending_project_opens` and `answer_project_open` read and answer a window's parked request: a map behind its own mutex, held for one lookup, and a oneshot send. - `note_window_revealed` records the window in a set behind its own mutex and, if a raise is owed, shows and focuses the native window, sets the PR poll focus hint and records the window as used. The window calls belong on the main thread and the rest is in memory. None reaches the store, a child process or the filesystem. They are not converted, so their ordering is unchanged and they needed no frontend audit. Ordering: the audit The command has one frontend call site: the `renameProject` wrapper, called by `handleRenameProject` in ProjectHome, which ProjectSection's inline title editor awaits and which then paints the name into projectsData with `projectRenamed`. - Successive renames of one project. The editor holds `renameProjectSaving` and disables its input, so one mounted editor sends one rename at a time. The flag is component state, though. Opening another project and coming back while a rename is still out mounts a fresh editor, which can send a second. As a synchronous command the first froze the window, so that could not happen. On the pool the two wait on the store lock together and the older name can land last and stay. Exposed, and only when a rename is stuck for seconds. - A read over the local paint. `projectRenamed` writes the name into the project list. A `list_projects` read sent before the rename landed may have read the store before it, and nothing refused it, so it put the old name back until the next list arrived. The usual case is a reload already in flight: the rename's own project-changed reload queues behind it and bumps no generation until it has been applied. Exposed, as a flicker. `list_projects` left the main thread in 8f6db6e, so this came with the rebase onto #1044 and not with this conversion. - Another ordered write for the same project. There is none: no `writeOrder` key is per project. The tag writes for a project go to other tables and commute with the name. Ordering: the fixes - `renameProject` sends through `writeOrder` under `project:<id>`, so a project's rename waits for the one sent before it and for nothing else. Only the command is ordered; the cache invalidation after it is not. - `projectRenamed` records a rename that really changes a name as a local write (`project-writes`). The two places that apply a project list, the reload and `refreshProject`, go through a new `admitProjectList`. A list sent before the rename is set aside, not applied, and the list is read again in the background, since it may carry some other change. The checks run in the order 8c519c1 settled on: refused, then set aside, then recorded, so a list that is set aside leaves no mark. Residual risk - A rename still in flight when its project is deleted. The delete is not sent behind the rename's key, so that a stuck rename cannot hold a delete back. If the delete's row removal won the store lock from a rename that was already waiting, the rename would fail on the missing project and leave its pinned error toast. The delete confirms in a dialog and takes the lock at least twice before it removes the row, so the rename lands first in practice. - A rename that never settles holds back later renames of that project. As a synchronous command it held back every command. - The order is per window, as for every other key. - `projectCreated` paints a new project into the same list without a mark. A list read sent before the create can drop the project until the next reload. Not new and not changed here. Verified - Pre-fix, on the tree rebased onto 485b576, with the pinned 1.99.1 toolchain: `cargo test --lib blocking_guard_tests` gives 3 passed, 1 failed, with the CI message. - CI run 43540379914 on c036996, which is this commit without the three list entries, merged with 22c8335. Rust lint and Frontend (prettier, svelte-check, tsc, vitest) pass. Rust test: 2271 passed, 1 failed. The failure is the same guard, now naming only #1072's three commands, so `rename_project` passes all four guards there. - Locally on c036996: `cargo fmt --check`, `cargo clippy --workspace -- -D warnings`, `pnpm check` (0 errors) and prettier on the changed frontend files pass. - The five new frontend tests pass: one for the wrapper order, three for the list guard, and one pinning that a rename that changes no name marks nothing. With the list guard disabled two of them fail; with the rename key made unique per call the wrapper test fails. The two test files they are in pass in full (151 tests). - After the second rebase and the list entries: `cargo fmt --check`. Not run, left to CI as asked - Any test after the second rebase and the three list entries. The guard has not been seen to pass with them; they were added from its failure message and from reading the commands. - The Rust test suite and the full vitest suite locally. - The app itself. Neither race was reproduced live. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
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.
Summary
Adds
stagedctl project open, a CLI verb that brings a project up in the running Staged app from a terminal, and makes thestagedlauncher a thin wrapper around it.stagedctl (
crates/stagedctl)project open [<project>] [--no-launch] [--json]verb (open.rs). A project is matched by exact id, exact name, then a unique case-insensitive name. With no argument the project is resolved from the current directory when it sits inside~/.staged/workspaces/<local|remote>/projects/<id>/; anywhere else fails withnot_in_workspace.Staged.appthe binary is bundled in and waits up to 30 s for it to answer (launch.rs).--no-launchexits 3 instead. Launch is skipped for dev builds, pinned lock paths, unauthorized callers and non-project directories.CredentialScope::Sessiondistinguishes an agent session's lease from a one-project grant so error wording no longer sends a session after a grant it cannot have.code: messagestderr lines are documented in the README and the bundledguide.md.Staged app (
src-tauri)open_projectcontrol command plus aproject_openmodule that parks the request as pending state, hands it to the frontend, and answers from the frontend's result (opened,ui_busy,ui_timeout). Read-only grants and session credentials are refused withscope_denied.show_a_windowis split into pick and raise helpers (window_use.rs,window_raise.rs). The window picked is the focused one, else the most recently focused one still open.note_window_revealedcommand (window_reveal.rs), so a cold-started window is raised exactly when its frontend has painted, with no early flash and no polling.stagedctl_linkmodule and Settings -> Connected apps -> Add to PATH action that symlinksstagedctlinto/usr/local/binor~/.local/bin, with explicit handling for existing links, non-link files and App Translocation.Frontend
projectOpenListeneranswers pending open requests by navigating to the project externally (openProjectExternallyinnavigation.svelte.ts), reporting busy UI (open dialog, unsaved diff comment) instead of switching.revealWindow()service wrapsshow()plus the reveal report;StagedctlPathSettingcomponent for the PATH action.Launcher and docs
scripts/stagedandinstall.shnow delegate tostagedctl project open. A bare word is always a project name;./../anything with a slash is a directory to run from.stagedctl connectand the exit-code table.Testing
cargo fmt --check,cargo clippy --workspace -- -D warnings,cargo test --workspace(app crate 2200 passed, stagedctl 184 passed, help fixture updated).pnpm run checkclean;vitest run239 files / 3598 tests passed.stagedctlfor bare words, path forms,$CDPATH, flags and exit-code passthrough.Note for reviewers: this adds backend types (
RevealedWindows, pending project-open state,CredentialScope::Session) and one Tauri command. Nothing is persisted and there is no schema change.🤖 Generated with Claude Code