diff --git a/CHANGELOG.md b/CHANGELOG.md index e0cf1a89..74a300b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 line to print, so it says that the recreate ends the agents and that `claude --resume` in the new container picks a conversation up again. No failure here refuses the recreate. See docs/cli.md, "A recreate starts the agents it ends again". +- **`dl --refresh-stale` recreates the stale workspaces that are idle** (#673). It runs + the recreate above, with no attach, on each workspace `dl --ls` calls stale, so it can + run after the daily pull. It skips a workspace, and says why, when another `dl` is + launching it, a build runs in it, an agent (`claude`, `codex` or `gemini`) runs a + command in it or is not idle, or it holds an agent session `dl` cannot start again + (outside herdr, any agent session). It prints its plan first and a count last, checks + each workspace again just before its recreate, and exits 1 when a recreate failed. + See docs/cli.md, "`--refresh-stale`: recreate the stale workspaces that are + idle". ## [0.60.1] - 2026-10-16 diff --git a/README.md b/README.md index bbcdf82e..d480d50c 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,7 @@ instead. [docs/cli.md](docs/cli.md) has the full `--rm` contract, including whic | `dl --prune` | Remove the clone directories no workspace opens any more, the agent git worktrees inside the clones it keeps, and the volumes and launch locks of workspaces devpod has forgotten | | `dl --reconcile` | Re-point workspaces whose recorded source folder went missing. Deletes nothing | | `dl --purge` | Remove devlaunch's own workspaces and caches | +| `dl --refresh-stale` | Recreate each workspace that runs an older image than its reference, when its agents are idle and nothing builds in it. Starts the agents again and skips the rest, saying why. See [docs/cli.md](docs/cli.md) | | `dl --install` | Install shell completions, and the `dl-herdr-shell` name a herdr pane opens through | | `dl --herdr-setup` | Install and configure the Herdr pane shell | | `dl --herdr-env set KEY=VALUE` | Save a variable for new host shells in this Herdr workspace | diff --git a/docs/cli.md b/docs/cli.md index 04af30b7..ad3fb6f0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -930,11 +930,19 @@ to bring each agent back (devlaunch#673): 2. After the `up` succeeds, `dl` types each pane's line back into that pane with `herdr pane run`. herdr refuses a pane until the old session in it has exited and its shell is back, so `dl` asks again every half second, for up to ten seconds per pane. -3. Then `dl` attaches to the new container as a recreate always did. +3. `dl` reports each line it typed to herdr again, as that pane's saved line, and reads + herdr's saved session to check that herdr kept it. The `dl` that the line runs saves + no line of its own, and herdr drops the old line when the old agent ends. Without + this step, the next recreate finds no line and ends the agent. herdr refuses the + report until it sees the new agent, so `dl` asks again every second, for up to + twenty seconds. All panes are asked at the same time. +4. Then `dl` attaches to the new container as a recreate always did. Each line is the one herdr saved for the pane, for example `dl -- ... claude ... --remote-control= --resume `. It holds the session -the pane is in now, because the container hook reports a new id after `/clear`. +the pane is in now, because the container hook reports a new id after `/clear`. An +agent started again from a line has no container hook, so after a `/clear` in it the +saved line still names the session it was started with. Nothing here can refuse the recreate. Each failure is one line on stderr, and the recreate goes ahead: @@ -948,10 +956,82 @@ recreate goes ahead: fails, gets the line printed, to run there by hand. - The pane that `dl` itself runs in is never typed into, because its foreground is that `dl`. Its line is printed instead. +- A pane whose line herdr did not keep gets its agent back, and `dl` says that the next + recreate ends it. + +#### A workspace on an image that devpod built + +With `"features"` (or anything else that makes devpod build), the container does not +run the reference itself but an image devpod built on it, tagged +`:devpod-`. devpod names that tag by a hash of the config, not of the +reference, and uses a tag it finds. So a recreate alone builds nothing, and the +workspace stays on the older image. + +When the workspace is stale (as `dl --ls` reports it), and its container runs a +`:devpod-` tag that differs from the reference, `dl recreate` first removes that +tag with `docker image rm --force `. devpod then builds the image again from the +newer reference. `--force` is necessary because the old container still uses the +image; with a tag, it removes only the tag, and the old container keeps its image until +the `up` replaces it. `dl` removes no tag that devpod did not make. If the removal +fails, `dl` says so and the recreate goes ahead on the older image. `reset` and `code` do none of this. A `reset` drops the volumes, and `code` keeps the container when nothing changed. +### `--refresh-stale`: recreate the stale workspaces that are idle + +`dl --ls` names each workspace whose container runs an older image than its reference +now names. `dl --refresh-stale` recreates those workspaces, but only the ones where a +recreate ends nothing that is in progress, and starts their agents again as +`dl recreate` does (devlaunch#673). It asks no question and attaches to nothing, +so it can run after the daily pull: + +```bash +docker pull ghcr.io/owner/image:latest && dl --refresh-stale +``` + +It prints one line for each stale workspace before it starts: `Refreshing `, or +`Skipping ` with the reason. Then it takes each workspace it did not skip in turn. +It checks the workspace again just before its recreate, because the recreates before +it can take minutes. If the workspace is now to be skipped, it prints +`Skipping after all` with the reason and leaves it alone. Otherwise it recreates +it, with the recreate's own lines. It ends with a count. A recreate that fails does not +stop the others. The exit status is 1 when a recreate failed, and 0 otherwise. A skip +is not a failure. + +`dl` skips a stale workspace when any of these is true, and names the first that is: + +- Another `dl` holds the workspace's launch lock. +- The container is stopped. A recreate would also start it, which a refresh does not + do. `dl recreate` starts it on the new image. +- `dl` cannot read the processes in the container. It reads them with one + `docker exec` as root. +- A build runs in the container: a process named `bazel`, `bazelisk`, `cargo`, `rustc`, + `cc1`, `cc1plus`, `make`, `gmake`, `ninja`, `cmake`, `colcon`, `gcc`, `g++`, + `clang`, `clang++`, `ld`, `ld.lld`, `ld.gold` or `collect2`. `pixi` is not on the + list: an open `pixi shell` is not a build, and a build that `pixi run` starts runs one + of the programs above. +- An agent process has a shell as its child. An agent is any of the agents `dl` + knows by name: `claude`, `codex` and `gemini`, run directly or by `node`. The agents + run their tool calls and background tasks through a shell, and Claude starts its MCP + servers directly, so a shell under an agent is work in progress and an MCP server + is not. +- An agent runs in the container and `dl` is not in a herdr pane, so it cannot see the + sessions to start them again. A container with no agent process needs no herdr. +- herdr does not answer, a pane holds a live agent and no saved line, or an agent is + not `idle` or `done` (it is `working`, or `blocked` on a permission prompt). +- More agent sessions run in the container than herdr's panes hold, counting agents + of every kind on both sides. A session that was started from a terminal outside + herdr has no line to start it again. + +A workspace on an image that devpod built is refreshed as `dl recreate` does it, +by removing devpod's tag first. If the removal fails, the refresh does not recreate +that workspace, because it would stay stale and be recreated again on every run. It +counts as a failed recreate. + +The second check is taken a moment before the recreate. An agent that starts work in +that moment ends with the recreate, the same as with `dl recreate`. + ## `kill`: the workspace that will not answer `dl stop` asks devpod to stop a workspace, and it is the right thing to type diff --git a/docs/development.md b/docs/development.md index c9c2c1c6..28f332ba 100644 --- a/docs/development.md +++ b/docs/development.md @@ -69,7 +69,7 @@ once under the `api` section and once under the module that owns them. **What it still does not reach, and it is not one type.** A type `api` never re-exports but a promised signature hands back is reachable from outside and classified as binary surface. Counted -on the checked-in files rather than guessed at, that is **40 types owning close to six hundred rows** in +on the checked-in files rather than guessed at, that is **41 types owning close to six hundred rows** in `public-api.rest.txt`, and the command that lists them needs no toolchain: **A falling count is not automatically a win, and it is worth knowing which kind you are looking diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index f356d09f..af5e009d 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -1478,6 +1478,10 @@ pane's saved directory. `dl` is what tells it the line, for an agent `aid` start dl -- CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 IS_SANDBOX=1 claude --dangerously-skip-permissions --remote-control= --resume ``` +The line starts with the `dl` that was run, by its file name, so a pane started with the +dev build `dl-next` (or `aid-next`, or `dla-next`) comes back under `dl-next` and not under +the released `dl`. + So after a herdr server restart, or a reboot, every such pane comes back into its own conversation with no `aid resume` and no pickers. The workspace starts if it is stopped. Measured against herdr 0.9.2: about 20 seconds from the server start to the claude diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 5f2f9eb8..8596cfd4 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -73,7 +73,7 @@ _dl_completion() { # The retired spellings (--stop, --autorm) are absent by rule rather than by # hand: the grammar marks them `hide = true`, and the test drops every hidden # flag, so a spelling this build only still answers for is never offered. - local global_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --herdr-setup --herdr-env --herdr-workspace --rm --devcontainer --claude-profile --claude-profiles --help -h --version" + local global_opts="--ls --install --refresh --refresh-stale --prune --reconcile --purge --herdr-shell --herdr-setup --herdr-env --herdr-workspace --rm --devcontainer --claude-profile --claude-profiles --help -h --version" if [[ "$cmd" == aid ]]; then global_opts="--claude --codex --gemini --model --effort --devcontainer --claude-profile --help -h --version" fi diff --git a/rust/devlaunch-core/public-api.api.txt b/rust/devlaunch-core/public-api.api.txt index 1402ed52..6c6a3991 100644 --- a/rust/devlaunch-core/public-api.api.txt +++ b/rust/devlaunch-core/public-api.api.txt @@ -36,6 +36,10 @@ impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::Ins pub enum devlaunch_core::api::LaunchNotice pub devlaunch_core::api::LaunchNotice::AgentSessionInThisPane pub devlaunch_core::api::LaunchNotice::AgentSessionInThisPane::line: alloc::string::String +pub devlaunch_core::api::LaunchNotice::AgentSessionLineNotKept +pub devlaunch_core::api::LaunchNotice::AgentSessionLineNotKept::pane_id: alloc::string::String +pub devlaunch_core::api::LaunchNotice::AgentSessionLineUnchecked +pub devlaunch_core::api::LaunchNotice::AgentSessionLineUnchecked::pane_id: alloc::string::String pub devlaunch_core::api::LaunchNotice::AgentSessionNotRestarted pub devlaunch_core::api::LaunchNotice::AgentSessionNotRestarted::line: alloc::string::String pub devlaunch_core::api::LaunchNotice::AgentSessionNotRestarted::pane_id: alloc::string::String @@ -72,6 +76,10 @@ pub devlaunch_core::api::LaunchNotice::CodexStageMissing pub devlaunch_core::api::LaunchNotice::CodexStageMissing::workspace_id: alloc::string::String pub devlaunch_core::api::LaunchNotice::CreateNeverFinished pub devlaunch_core::api::LaunchNotice::CreateNeverFinished::workspace_id: alloc::string::String +pub devlaunch_core::api::LaunchNotice::DerivedImageKept(devlaunch_core::flows::launch::DerivedImageKept) +pub devlaunch_core::api::LaunchNotice::DerivedImageRemoved +pub devlaunch_core::api::LaunchNotice::DerivedImageRemoved::image: alloc::string::String +pub devlaunch_core::api::LaunchNotice::DerivedImageRemoved::workspace_id: alloc::string::String pub devlaunch_core::api::LaunchNotice::DevcontainerIgnoredRunning pub devlaunch_core::api::LaunchNotice::DevcontainerIgnoredRunning::spec: alloc::string::String pub devlaunch_core::api::LaunchNotice::DevcontainerIgnoredRunning::workspace_id: alloc::string::String @@ -142,6 +150,7 @@ pub devlaunch_core::api::LaunchVerb::Attach::command: core::option::Option core::option::Option<&'static str> pub fn devlaunch_core::clients::herdr_binary_from_process() -> core::option::Option pub fn devlaunch_core::clients::herdr_is_assignment(&str) -> bool +pub fn devlaunch_core::clients::herdr_resume_line(&str) -> core::option::Option +pub fn devlaunch_core::clients::herdr_resume_lines() -> alloc::vec::Vec pub mod devlaunch_core::domain pub mod devlaunch_core::domain::config pub enum devlaunch_core::domain::config::ConfigError @@ -850,6 +852,8 @@ impl core::marker::StructuralPartialEq for devlaunch_core::flows::agent_sessions pub struct devlaunch_core::flows::agent_sessions::HeldSession pub devlaunch_core::flows::agent_sessions::HeldSession::line: alloc::vec::Vec pub devlaunch_core::flows::agent_sessions::HeldSession::pane_id: alloc::string::String +impl devlaunch_core::flows::agent_sessions::HeldSession +pub fn devlaunch_core::flows::agent_sessions::HeldSession::shell_line(&self) -> alloc::string::String impl core::clone::Clone for devlaunch_core::flows::agent_sessions::HeldSession pub fn devlaunch_core::flows::agent_sessions::HeldSession::clone(&self) -> devlaunch_core::flows::agent_sessions::HeldSession impl core::cmp::Eq for devlaunch_core::flows::agent_sessions::HeldSession @@ -871,17 +875,17 @@ pub fn devlaunch_core::flows::agent_sessions::HeldSessions::default() -> devlaun impl core::fmt::Debug for devlaunch_core::flows::agent_sessions::HeldSessions pub fn devlaunch_core::flows::agent_sessions::HeldSessions::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::agent_sessions::HeldSessions -pub struct devlaunch_core::flows::agent_sessions::Manager -impl devlaunch_core::flows::agent_sessions::Manager -pub fn devlaunch_core::flows::agent_sessions::Manager::from_host(&devlaunch_core::flows::launch::Host) -> core::option::Option -impl core::clone::Clone for devlaunch_core::flows::agent_sessions::Manager -pub fn devlaunch_core::flows::agent_sessions::Manager::clone(&self) -> devlaunch_core::flows::agent_sessions::Manager -impl core::cmp::Eq for devlaunch_core::flows::agent_sessions::Manager -impl core::cmp::PartialEq for devlaunch_core::flows::agent_sessions::Manager -pub fn devlaunch_core::flows::agent_sessions::Manager::eq(&self, &devlaunch_core::flows::agent_sessions::Manager) -> bool -impl core::fmt::Debug for devlaunch_core::flows::agent_sessions::Manager -pub fn devlaunch_core::flows::agent_sessions::Manager::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl core::marker::StructuralPartialEq for devlaunch_core::flows::agent_sessions::Manager +pub struct devlaunch_core::flows::agent_sessions::HerdrPanes +impl devlaunch_core::flows::agent_sessions::HerdrPanes +pub fn devlaunch_core::flows::agent_sessions::HerdrPanes::from_host(&devlaunch_core::flows::launch::Host) -> core::option::Option +impl core::clone::Clone for devlaunch_core::flows::agent_sessions::HerdrPanes +pub fn devlaunch_core::flows::agent_sessions::HerdrPanes::clone(&self) -> devlaunch_core::flows::agent_sessions::HerdrPanes +impl core::cmp::Eq for devlaunch_core::flows::agent_sessions::HerdrPanes +impl core::cmp::PartialEq for devlaunch_core::flows::agent_sessions::HerdrPanes +pub fn devlaunch_core::flows::agent_sessions::HerdrPanes::eq(&self, &devlaunch_core::flows::agent_sessions::HerdrPanes) -> bool +impl core::fmt::Debug for devlaunch_core::flows::agent_sessions::HerdrPanes +pub fn devlaunch_core::flows::agent_sessions::HerdrPanes::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::agent_sessions::HerdrPanes pub struct devlaunch_core::flows::agent_sessions::UnresumablePane pub devlaunch_core::flows::agent_sessions::UnresumablePane::agent: alloc::string::String pub devlaunch_core::flows::agent_sessions::UnresumablePane::pane_id: alloc::string::String @@ -893,9 +897,8 @@ pub fn devlaunch_core::flows::agent_sessions::UnresumablePane::eq(&self, &devlau impl core::fmt::Debug for devlaunch_core::flows::agent_sessions::UnresumablePane pub fn devlaunch_core::flows::agent_sessions::UnresumablePane::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::agent_sessions::UnresumablePane -pub fn devlaunch_core::flows::agent_sessions::agent_reading(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::agent_sessions::Manager, &str) -> devlaunch_core::clients::herdr::AgentReading -pub fn devlaunch_core::flows::agent_sessions::collect(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::agent_sessions::Manager, &str, &dyn core::ops::function::Fn(&std::path::Path) -> core::option::Option) -> core::option::Option -pub fn devlaunch_core::flows::agent_sessions::restart(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::agent_sessions::Manager, &devlaunch_core::flows::agent_sessions::HeldSessions, &dyn core::ops::function::Fn(core::time::Duration)) -> alloc::vec::Vec +pub fn devlaunch_core::flows::agent_sessions::collect(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::agent_sessions::HerdrPanes, &str, &dyn core::ops::function::Fn(&std::path::Path) -> core::option::Option) -> core::option::Option +pub fn devlaunch_core::flows::agent_sessions::restart(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::agent_sessions::HerdrPanes, &devlaunch_core::flows::agent_sessions::HeldSessions, &dyn core::ops::function::Fn(core::time::Duration)) -> alloc::vec::Vec pub mod devlaunch_core::flows::agent_worktrees pub enum devlaunch_core::flows::agent_worktrees::AgentOwner pub devlaunch_core::flows::agent_worktrees::AgentOwner::CouldNotTell(alloc::string::String) @@ -1882,6 +1885,7 @@ pub devlaunch_core::flows::launch::LaunchRefusal::BranchNotNamed pub devlaunch_core::flows::launch::LaunchRefusal::BranchNotNamed::error: devlaunch_core::flows::launch::BranchNotNamed pub devlaunch_core::flows::launch::LaunchRefusal::BranchNotNamed::owner: alloc::string::String pub devlaunch_core::flows::launch::LaunchRefusal::BranchNotNamed::repo: alloc::string::String +pub devlaunch_core::flows::launch::LaunchRefusal::DerivedImageKept(devlaunch_core::flows::launch::DerivedImageKept) pub devlaunch_core::flows::launch::LaunchRefusal::IdCollision pub devlaunch_core::flows::launch::LaunchRefusal::IdCollision::branch: alloc::string::String pub devlaunch_core::flows::launch::LaunchRefusal::IdCollision::owner: alloc::string::String @@ -2051,6 +2055,18 @@ pub fn devlaunch_core::flows::launch::TerminalTitle::eq(&self, &devlaunch_core:: impl core::fmt::Debug for devlaunch_core::flows::launch::TerminalTitle pub fn devlaunch_core::flows::launch::TerminalTitle::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::launch::TerminalTitle +pub struct devlaunch_core::flows::launch::DerivedImageKept +pub devlaunch_core::flows::launch::DerivedImageKept::image: alloc::string::String +pub devlaunch_core::flows::launch::DerivedImageKept::why: alloc::string::String +pub devlaunch_core::flows::launch::DerivedImageKept::workspace_id: alloc::string::String +impl core::clone::Clone for devlaunch_core::flows::launch::DerivedImageKept +pub fn devlaunch_core::flows::launch::DerivedImageKept::clone(&self) -> devlaunch_core::flows::launch::DerivedImageKept +impl core::cmp::Eq for devlaunch_core::flows::launch::DerivedImageKept +impl core::cmp::PartialEq for devlaunch_core::flows::launch::DerivedImageKept +pub fn devlaunch_core::flows::launch::DerivedImageKept::eq(&self, &devlaunch_core::flows::launch::DerivedImageKept) -> bool +impl core::fmt::Debug for devlaunch_core::flows::launch::DerivedImageKept +pub fn devlaunch_core::flows::launch::DerivedImageKept::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::launch::DerivedImageKept pub struct devlaunch_core::flows::launch::UnquotableCommand pub devlaunch_core::flows::launch::UnquotableCommand::command: alloc::string::String impl core::clone::Clone for devlaunch_core::flows::launch::UnquotableCommand @@ -3039,6 +3055,97 @@ pub fn devlaunch_core::flows::pull_request::resolve(&dyn devlaunch_runner::Runne pub mod devlaunch_core::flows::records pub fn devlaunch_core::flows::records::open_records<'r>(&'r dyn devlaunch_runner::Runner) -> core::result::Result, devlaunch_core::flows::records::StartupError> pub fn devlaunch_core::flows::records::open_storage() -> core::result::Result<(devlaunch_core::domain::metadata::MetadataStorage, alloc::vec::Vec), devlaunch_core::flows::records::StartupError> +pub mod devlaunch_core::flows::refresh_stale +pub enum devlaunch_core::flows::refresh_stale::AgentState +pub devlaunch_core::flows::refresh_stale::AgentState::Blocked +pub devlaunch_core::flows::refresh_stale::AgentState::Unanswered +pub devlaunch_core::flows::refresh_stale::AgentState::Unknown +pub devlaunch_core::flows::refresh_stale::AgentState::Working +impl core::clone::Clone for devlaunch_core::flows::refresh_stale::AgentState +pub fn devlaunch_core::flows::refresh_stale::AgentState::clone(&self) -> devlaunch_core::flows::refresh_stale::AgentState +impl core::cmp::Eq for devlaunch_core::flows::refresh_stale::AgentState +impl core::cmp::PartialEq for devlaunch_core::flows::refresh_stale::AgentState +pub fn devlaunch_core::flows::refresh_stale::AgentState::eq(&self, &devlaunch_core::flows::refresh_stale::AgentState) -> bool +impl core::fmt::Debug for devlaunch_core::flows::refresh_stale::AgentState +pub fn devlaunch_core::flows::refresh_stale::AgentState::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for devlaunch_core::flows::refresh_stale::AgentState +impl core::marker::StructuralPartialEq for devlaunch_core::flows::refresh_stale::AgentState +pub enum devlaunch_core::flows::refresh_stale::Skip +pub devlaunch_core::flows::refresh_stale::Skip::AgentBusy +pub devlaunch_core::flows::refresh_stale::Skip::AgentBusy::pane_id: alloc::string::String +pub devlaunch_core::flows::refresh_stale::Skip::AgentBusy::state: devlaunch_core::flows::refresh_stale::AgentState +pub devlaunch_core::flows::refresh_stale::Skip::AgentRunsACommand +pub devlaunch_core::flows::refresh_stale::Skip::AgentRunsACommand::pid: u32 +pub devlaunch_core::flows::refresh_stale::Skip::AgentRunsACommand::program: alloc::string::String +pub devlaunch_core::flows::refresh_stale::Skip::Building +pub devlaunch_core::flows::refresh_stale::Skip::Building::pid: u32 +pub devlaunch_core::flows::refresh_stale::Skip::Building::program: alloc::string::String +pub devlaunch_core::flows::refresh_stale::Skip::HerdrUnanswered +pub devlaunch_core::flows::refresh_stale::Skip::LaunchUnderWay +pub devlaunch_core::flows::refresh_stale::Skip::LockUnreadable +pub devlaunch_core::flows::refresh_stale::Skip::LockUnreadable::why: alloc::string::String +pub devlaunch_core::flows::refresh_stale::Skip::ProcessesUnread +pub devlaunch_core::flows::refresh_stale::Skip::ProcessesUnread::why: alloc::string::String +pub devlaunch_core::flows::refresh_stale::Skip::SessionsOutsidePanes +pub devlaunch_core::flows::refresh_stale::Skip::SessionsOutsidePanes::held: usize +pub devlaunch_core::flows::refresh_stale::Skip::SessionsOutsidePanes::running: usize +pub devlaunch_core::flows::refresh_stale::Skip::SessionsUnseen +pub devlaunch_core::flows::refresh_stale::Skip::Stopped +pub devlaunch_core::flows::refresh_stale::Skip::Unresumable +pub devlaunch_core::flows::refresh_stale::Skip::Unresumable::pane_id: alloc::string::String +impl core::clone::Clone for devlaunch_core::flows::refresh_stale::Skip +pub fn devlaunch_core::flows::refresh_stale::Skip::clone(&self) -> devlaunch_core::flows::refresh_stale::Skip +impl core::cmp::Eq for devlaunch_core::flows::refresh_stale::Skip +impl core::cmp::PartialEq for devlaunch_core::flows::refresh_stale::Skip +pub fn devlaunch_core::flows::refresh_stale::Skip::eq(&self, &devlaunch_core::flows::refresh_stale::Skip) -> bool +impl core::fmt::Debug for devlaunch_core::flows::refresh_stale::Skip +pub fn devlaunch_core::flows::refresh_stale::Skip::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::refresh_stale::Skip +pub enum devlaunch_core::flows::refresh_stale::Verdict +pub devlaunch_core::flows::refresh_stale::Verdict::Refresh +pub devlaunch_core::flows::refresh_stale::Verdict::Skip(devlaunch_core::flows::refresh_stale::Skip) +impl core::clone::Clone for devlaunch_core::flows::refresh_stale::Verdict +pub fn devlaunch_core::flows::refresh_stale::Verdict::clone(&self) -> devlaunch_core::flows::refresh_stale::Verdict +impl core::cmp::Eq for devlaunch_core::flows::refresh_stale::Verdict +impl core::cmp::PartialEq for devlaunch_core::flows::refresh_stale::Verdict +pub fn devlaunch_core::flows::refresh_stale::Verdict::eq(&self, &devlaunch_core::flows::refresh_stale::Verdict) -> bool +impl core::fmt::Debug for devlaunch_core::flows::refresh_stale::Verdict +pub fn devlaunch_core::flows::refresh_stale::Verdict::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::refresh_stale::Verdict +pub struct devlaunch_core::flows::refresh_stale::Planned +pub devlaunch_core::flows::refresh_stale::Planned::reference: alloc::string::String +pub devlaunch_core::flows::refresh_stale::Planned::verdict: devlaunch_core::flows::refresh_stale::Verdict +pub devlaunch_core::flows::refresh_stale::Planned::workspace_id: alloc::string::String +impl core::clone::Clone for devlaunch_core::flows::refresh_stale::Planned +pub fn devlaunch_core::flows::refresh_stale::Planned::clone(&self) -> devlaunch_core::flows::refresh_stale::Planned +impl core::cmp::Eq for devlaunch_core::flows::refresh_stale::Planned +impl core::cmp::PartialEq for devlaunch_core::flows::refresh_stale::Planned +pub fn devlaunch_core::flows::refresh_stale::Planned::eq(&self, &devlaunch_core::flows::refresh_stale::Planned) -> bool +impl core::fmt::Debug for devlaunch_core::flows::refresh_stale::Planned +pub fn devlaunch_core::flows::refresh_stale::Planned::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::refresh_stale::Planned +pub struct devlaunch_core::flows::refresh_stale::Tally +pub devlaunch_core::flows::refresh_stale::Tally::failed: usize +pub devlaunch_core::flows::refresh_stale::Tally::refreshed: usize +pub devlaunch_core::flows::refresh_stale::Tally::skipped: usize +impl core::clone::Clone for devlaunch_core::flows::refresh_stale::Tally +pub fn devlaunch_core::flows::refresh_stale::Tally::clone(&self) -> devlaunch_core::flows::refresh_stale::Tally +impl core::cmp::Eq for devlaunch_core::flows::refresh_stale::Tally +impl core::cmp::PartialEq for devlaunch_core::flows::refresh_stale::Tally +pub fn devlaunch_core::flows::refresh_stale::Tally::eq(&self, &devlaunch_core::flows::refresh_stale::Tally) -> bool +impl core::default::Default for devlaunch_core::flows::refresh_stale::Tally +pub fn devlaunch_core::flows::refresh_stale::Tally::default() -> devlaunch_core::flows::refresh_stale::Tally +impl core::fmt::Debug for devlaunch_core::flows::refresh_stale::Tally +pub fn devlaunch_core::flows::refresh_stale::Tally::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for devlaunch_core::flows::refresh_stale::Tally +impl core::marker::StructuralPartialEq for devlaunch_core::flows::refresh_stale::Tally +pub const devlaunch_core::flows::refresh_stale::BUILD_PROGRAMS: &[&str] +pub trait devlaunch_core::flows::refresh_stale::Refresher +pub fn devlaunch_core::flows::refresh_stale::Refresher::left_alone(&mut self, &str, &devlaunch_core::flows::refresh_stale::Skip) +pub fn devlaunch_core::flows::refresh_stale::Refresher::planned(&mut self, &[devlaunch_core::flows::refresh_stale::Planned]) +pub fn devlaunch_core::flows::refresh_stale::Refresher::recreate(&mut self, &str) -> bool +pub fn devlaunch_core::flows::refresh_stale::plan<'w>(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::stale_images::StaleImages, impl core::iter::traits::collect::IntoIterator, core::option::Option<&devlaunch_core::flows::agent_sessions::HerdrPanes>, &devlaunch_core::flows::launch_locks::LaunchLocks, &dyn core::ops::function::Fn(&std::path::Path) -> core::option::Option) -> alloc::vec::Vec +pub fn devlaunch_core::flows::refresh_stale::run<'w>(&dyn devlaunch_runner::Runner, &devlaunch_core::flows::stale_images::StaleImages, impl core::iter::traits::collect::IntoIterator, core::option::Option<&devlaunch_core::flows::agent_sessions::HerdrPanes>, &devlaunch_core::flows::launch_locks::LaunchLocks, &dyn core::ops::function::Fn(&std::path::Path) -> core::option::Option, &mut dyn devlaunch_core::flows::refresh_stale::Refresher) -> devlaunch_core::flows::refresh_stale::Tally pub mod devlaunch_core::flows::repo_manager pub enum devlaunch_core::flows::repo_manager::CacheNotice pub devlaunch_core::flows::repo_manager::CacheNotice::AdoptedBareClone @@ -3262,6 +3369,8 @@ pub fn devlaunch_core::flows::session_manager::pane_destination(&dyn devlaunch_r pub mod devlaunch_core::flows::stale_images pub struct devlaunch_core::flows::stale_images::StaleImage impl devlaunch_core::flows::stale_images::StaleImage +pub fn devlaunch_core::flows::stale_images::StaleImage::container(&self) -> &str +pub fn devlaunch_core::flows::stale_images::StaleImage::derived_image(&self) -> core::option::Option<&str> pub fn devlaunch_core::flows::stale_images::StaleImage::reference(&self) -> &str impl core::clone::Clone for devlaunch_core::flows::stale_images::StaleImage pub fn devlaunch_core::flows::stale_images::StaleImage::clone(&self) -> devlaunch_core::flows::stale_images::StaleImage diff --git a/rust/devlaunch-core/src/clients/docker.rs b/rust/devlaunch-core/src/clients/docker.rs index 4b83d27d..ea69e833 100644 --- a/rust/devlaunch-core/src/clients/docker.rs +++ b/rust/devlaunch-core/src/clients/docker.rs @@ -89,6 +89,18 @@ pub(crate) fn remove_volumes(runner: &dyn Runner, names: &NonEmpty) -> A answer(runner.capture(&spec)) } +/// Remove one tag, and the image under it only when nothing uses that image. +/// +/// `--force` because the container that runs the image is still there when this +/// is asked: without it docker refuses the tag of an image any container uses. +/// With it, and a tag rather than an id, docker removes the tag and keeps the +/// image for the container. Callers pass only a tag dl knows devpod made. +pub(crate) fn untag_image(runner: &dyn Runner, reference: &str) -> Answer { + let spec = + SpawnSpec::from(Invocation::new(PROGRAM).with_args(["image", "rm", "--force", reference])); + answer(runner.capture(&spec)) +} + /// The containers one compose project has running, or why docker could not say. /// /// Its own type rather than an [`Answer`] carrying stdout, because this is the @@ -211,10 +223,38 @@ pub(crate) fn exec_as_root( runner: &dyn Runner, container: &str, argv: &[&str], -) -> Result { +) -> Result { let mut args = vec!["exec", "--user", "0", container]; args.extend_from_slice(argv); - read(runner, &args, READ_INSIDE) + read_or(runner, &args, READ_INSIDE, |stderr| { + // docker's words for an exec into a stopped container, stable since + // docker 1.x: "container is not running". + stderr + .contains("is not running") + .then_some(ExecFailure::NotRunning) + }) + .map_err(|failure| match failure { + Refused::Known(failure) => failure, + Refused::Words(why) => ExecFailure::Other(why), + }) +} + +/// Why [`exec_as_root`] printed nothing to read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ExecFailure { + /// The container is not running, so there is nothing inside it to ask. + NotRunning, + /// Any other failure, in words for a person. + Other(String), +} + +impl std::fmt::Display for ExecFailure { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotRunning => f.write_str("the container is not running"), + Self::Other(why) => f.write_str(why), + } + } } /// A `--format`ted `docker inspect` over these names: what it printed, whatever @@ -247,23 +287,52 @@ pub(crate) fn inspect_formatted( /// One bounded docker read: its stdout when it exited 0, and otherwise words /// for why not. fn read(runner: &dyn Runner, args: &[&str], timeout: Duration) -> Result { + read_or(runner, args, timeout, |_| None::).map_err(|failure| { + match failure { + Refused::Known(never) => match never {}, + Refused::Words(why) => why, + } + }) +} + +/// Why [`read_or`] has no stdout to give: a refusal the caller knows by its +/// stderr, or words for any other failure. +enum Refused { + Known(K), + Words(String), +} + +/// [`read`], with `known` asked first about the stderr of a docker that ran and +/// failed, so a refusal the caller tells apart is told apart here, next to the +/// call that printed it. +fn read_or( + runner: &dyn Runner, + args: &[&str], + timeout: Duration, + known: impl FnOnce(&str) -> Option, +) -> Result> { let spec = SpawnSpec::from(Invocation::new(PROGRAM).with_args(args.iter().copied())) .with_timeout(timeout); let verb = args.first().copied().unwrap_or_default(); match ran(runner.capture(&spec)) { Ok((exit, CapturedText { stdout, .. })) if exit.is_success() => Ok(stdout), - Ok((exit, CapturedText { stderr, .. })) => Err(format!( - "docker {verb} failed ({exit:?}): {}", - stderr.trim() + Ok((exit, CapturedText { stderr, .. })) => Err(known(&stderr).map_or_else( + || { + Refused::Words(format!( + "docker {verb} failed ({exit:?}): {}", + stderr.trim() + )) + }, + Refused::Known, )), - Err(NoAnswer::NotInstalled) => Err("docker is not installed".to_owned()), - Err(NoAnswer::NotStarted(failure)) if failure.kind == ErrorKind::TimedOut => { - Err(format!("docker {verb} did not answer in time")) - } - Err(NoAnswer::NotStarted(failure)) => Err(format!( + Err(NoAnswer::NotInstalled) => Err(Refused::Words("docker is not installed".to_owned())), + Err(NoAnswer::NotStarted(failure)) if failure.kind == ErrorKind::TimedOut => Err( + Refused::Words(format!("docker {verb} did not answer in time")), + ), + Err(NoAnswer::NotStarted(failure)) => Err(Refused::Words(format!( "docker {verb} could not start ({:?})", failure.kind - )), + ))), } } @@ -432,6 +501,39 @@ mod tests { ); } + /// docker's own words for an exec into a stopped container, as it prints + /// them, read as a container that is not running rather than as a fault. + #[test] + fn an_exec_into_a_stopped_container_says_it_is_not_running() { + let fake = FakeRunner::new(); + fake.script( + ["docker", "exec"], + Response::failed( + 1, + "Error response from daemon: container 4f1c2a9be0d3 is not running\n", + ), + ); + + assert_eq!( + exec_as_root(&fake, "4f1c2a9be0d3", &["true"]), + Err(ExecFailure::NotRunning) + ); + } + + #[test] + fn an_exec_docker_refused_otherwise_keeps_its_words() { + let fake = FakeRunner::new(); + fake.script( + ["docker", "exec"], + Response::failed(1, "Error response from daemon: permission denied\n"), + ); + + let Err(ExecFailure::Other(why)) = exec_as_root(&fake, "c1", &["true"]) else { + panic!("not an Other"); + }; + assert!(why.contains("permission denied"), "{why}"); + } + #[test] fn a_docker_that_refused_answers_with_its_own_words() { let fake = FakeRunner::new(); diff --git a/rust/devlaunch-core/src/clients/herdr.rs b/rust/devlaunch-core/src/clients/herdr.rs index 70cf5279..78ea81b3 100644 --- a/rust/devlaunch-core/src/clients/herdr.rs +++ b/rust/devlaunch-core/src/clients/herdr.rs @@ -48,6 +48,36 @@ pub(crate) const AGENT_VAR: &str = "HERDR_AGENT"; /// silence dl gives today. pub(crate) const AGENT_NAMES: &[&str] = &["claude", "codex", "gemini"]; +/// The words after each agent's program that pick its last conversation up again, +/// one row per name in [`AGENT_NAMES`]. aid's `AGENTS` table spells the same +/// words as each row's `resume`, and `test/unit/test_session_manager.py` diffs +/// the two. +const AGENT_RESUMES: &[(&str, &str)] = &[ + ("claude", "--resume"), + ("codex", "resume"), + ("gemini", "--resume"), +]; + +/// The line a user types to pick a known agent's last conversation up again, such +/// as `codex resume`. `None` for a program that is not an agent dl knows. +/// Binary surface: `dl` names it when a recreate ends an agent it cannot restart. +pub fn resume_line(agent: &str) -> Option { + let agent = agent_named(agent)?; + AGENT_RESUMES + .iter() + .find(|(name, _)| *name == agent) + .map(|(name, resume)| format!("{name} {resume}")) +} + +/// [`resume_line`] for every agent dl knows, in [`AGENT_NAMES`] order. +/// Binary surface: see [`resume_line`]. +pub fn resume_lines() -> Vec { + AGENT_NAMES + .iter() + .filter_map(|name| resume_line(name)) + .collect() +} + /// The agent a workspace command starts, when its program is one by name. /// /// Reads the command the way a shell would begin to: leading `NAME=value` @@ -146,6 +176,21 @@ fn inherited(env: EnvSpec) -> EnvSpec { mod tests { use super::*; + #[test] + fn each_known_agent_resumes_in_its_own_words() { + assert_eq!(resume_line("claude").as_deref(), Some("claude --resume")); + assert_eq!(resume_line("codex").as_deref(), Some("codex resume")); + assert_eq!( + resume_line("/usr/local/bin/gemini").as_deref(), + Some("gemini --resume") + ); + assert_eq!(resume_line("make"), None); + assert_eq!( + resume_lines(), + ["claude --resume", "codex resume", "gemini --resume"] + ); + } + #[test] fn an_agent_command_names_its_agent() { assert_eq!(agent_in("claude"), Some("claude")); @@ -1807,7 +1852,7 @@ pub enum AgentStatus { Working, /// Waiting on a permission prompt. Blocked, - /// The agent has ended. herdr keeps a finished agent queryable in its pane. + /// The agent finished its turn and nobody has looked at it yet. It is idle, not ended. Done, Unknown, } @@ -1937,11 +1982,12 @@ fn herdr_accepts(argv: &[String]) -> bool { /// `dl [ --claude-profile ] -- `, as herdr will type it. fn restore_line<'a>( + program: &str, workspace_id: &str, profile: Option<&str>, words: impl Iterator, ) -> Vec { - let mut argv = vec![RESTORE_PROGRAM.to_owned(), workspace_id.to_owned()]; + let mut argv = vec![program.to_owned(), workspace_id.to_owned()]; if let Some(profile) = profile { argv.push("--claude-profile".to_owned()); argv.push(profile.to_owned()); @@ -1951,17 +1997,59 @@ fn restore_line<'a>( argv } -/// The program a restored pane runs: `dl` by name, which is also herdr's rule. +/// The program a restored pane runs: the `dl` this process was started as, by +/// name, which is also herdr's rule. /// -/// By name and not by path for the pane shell's reason (`dl-herdr-shell`): the -/// directory a path names is replaced by `pixi global update`. +/// `invoked_as` is argv\[0\] as given ([`crate::flows::launch::Host`]'s +/// `invoked_as`), so a dev build (`dl-next`) resumes under itself and not under +/// the released `dl` (devlaunch#677). Its file name only, for the pane shell's +/// reason (`dl-herdr-shell`): the directory a path names is replaced by +/// `pixi global update`, and herdr refuses a path anyway. Not `current_exe()`, +/// which resolves pixi global's trampoline to a path that moves on upgrade. +/// `aid` runs dl in process, so its `aid-next` resumes under `dl-next`, and +/// `dla`, which is aid under a second name, maps the same way. A name a +/// line cannot carry (empty, an option, a space or an apostrophe) falls back to +/// `dl`. Pinned by `a_restored_pane_runs_the_dl_that_was_started`. +pub(crate) fn restore_program(invoked_as: Option<&str>) -> String { + let name = invoked_as + .and_then(|path| path.rsplit('/').next()) + .map(|name| { + match name + .strip_prefix("aid") + .or_else(|| name.strip_prefix("dla")) + { + Some(rest) => format!("{RESTORE_PROGRAM}{rest}"), + None => name.to_owned(), + } + }); + match name { + Some(name) + if !name.is_empty() + && !name.starts_with('-') + && !name.contains('\'') + && !name.contains(char::is_whitespace) => + { + name + } + _ => RESTORE_PROGRAM.to_owned(), + } +} + +/// The released `dl`, which a restored pane runs when nothing better is known. const RESTORE_PROGRAM: &str = "dl"; impl AgentResume { /// The argv herdr types into the restored pane, or `None` when herdr would /// refuse it. - pub(crate) fn argv(&self, workspace_id: &str, profile: Option<&str>) -> Option> { - let argv = restore_line(workspace_id, profile, self.line.iter()); + /// + /// `program` is [`restore_program`]'s answer. + pub(crate) fn argv( + &self, + program: &str, + workspace_id: &str, + profile: Option<&str>, + ) -> Option> { + let argv = restore_line(program, workspace_id, profile, self.line.iter()); herdr_accepts(&argv).then_some(argv) } @@ -1971,9 +2059,14 @@ impl AgentResume { /// apostrophe, because the hook can split on neither safely. Checked with a /// UUID's worth of id on the end, so a line the id would push over herdr's /// limits is not handed to a hook that will then be refused. - pub(crate) fn hook_prefix(&self, workspace_id: &str, profile: Option<&str>) -> Option { + pub(crate) fn hook_prefix( + &self, + program: &str, + workspace_id: &str, + profile: Option<&str>, + ) -> Option { let by_id = self.by_id.as_ref()?; - let prefix = restore_line(workspace_id, profile, by_id.iter()); + let prefix = restore_line(program, workspace_id, profile, by_id.iter()); if prefix .iter() .any(|word| word.is_empty() || word.contains(char::is_whitespace)) @@ -1986,6 +2079,38 @@ impl AgentResume { } } +/// The herdr this process runs in a pane of, and where it saves its session. +/// +/// One rule for both the per-pane report ([`ResumeReport`]) and the recreate's +/// scan of every pane (`flows::agent_sessions`): provisioning has to be off for +/// the pane, the socket's directory is where herdr keeps `session.json`, and the +/// binary falls back to a `PATH` lookup, the rule every other herdr call in dl +/// follows. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Locator { + pub(crate) binary: String, + /// The pane this process runs in, when herdr named one. + pub(crate) pane_id: Option, + pub(crate) session_file: PathBuf, +} + +impl Locator { + /// The herdr `host` describes, or `None` outside one of its panes. + pub(crate) fn resolve(host: &HostEnv, binary: Option<&str>) -> Option { + if !crate::flows::provision::provisioning_disabled(host.in_pane.as_deref()) { + return None; + } + let socket = non_empty(host.socket.as_deref())?; + let session_file = Path::new(&socket).parent()?.join("session.json"); + Some(Self { + binary: non_empty(binary) + .unwrap_or_else(|| crate::flows::launch::HERDR_BIN_FALLBACK.to_owned()), + pane_id: non_empty(host.pane_id.as_deref()), + session_file, + }) + } +} + /// One pane to report a resume argv for, from the host. /// /// No consent is asked for, unlike [`Reporting`]: this costs one herdr call from @@ -2012,22 +2137,40 @@ impl ResumeReport { agent: Option<&'static str>, argv: Option>, ) -> Option { - if !crate::flows::provision::provisioning_disabled(host.in_pane.as_deref()) { - return None; - } - let pane_id = non_empty(host.pane_id.as_deref())?; - let socket = non_empty(host.socket.as_deref())?; - let session_file = Path::new(&socket).parent()?.join("session.json"); - Some(Self { - binary: non_empty(binary) - .unwrap_or_else(|| crate::flows::launch::HERDR_BIN_FALLBACK.to_owned()), + let Locator { + binary, pane_id, session_file, + } = Locator::resolve(host, binary)?; + Some(Self { + binary, + pane_id: pane_id?, + session_file, agent: agent?, argv: argv?, }) } + /// The report for a line that is already known, and the pane it belongs to. + /// + /// For the agent a recreate starts again: the `dl` that line runs is given no + /// line of its own to report, so the recreate reports the one it typed. + pub(crate) fn for_pane( + binary: &str, + pane_id: &str, + session_file: &Path, + agent: &'static str, + argv: Vec, + ) -> Self { + Self { + binary: binary.to_owned(), + pane_id: pane_id.to_owned(), + session_file: session_file.to_owned(), + agent, + argv, + } + } + pub(crate) fn binary(&self) -> &str { &self.binary } @@ -2100,7 +2243,11 @@ pub(crate) enum Saved { /// an internal number that `public_pane_numbers` maps to the public one. pub(crate) fn saved_argv(session_json: &str, pane_id: &str) -> Option>> { let (workspace_id, public) = pane_id.split_once(":p")?; - let public: u64 = public.parse().ok()?; + let public: u64 = public.chars().try_fold(0u64, |n, c| { + "0123456789ABCDEFGHJKMNPQRSTVWXYZ" + .find(c.to_ascii_uppercase()) + .map(|d| n * 32 + d as u64) + })?; let session: serde_json::Value = serde_json::from_str(session_json).ok()?; let Some(workspace) = session .get("workspaces")? @@ -2176,7 +2323,7 @@ mod resume_tests { #[test] fn a_restored_pane_runs_dl_on_the_workspace_it_left() { assert_eq!( - claude_resume().argv("ws-id", None), + claude_resume().argv("dl", "ws-id", None), Some( [ "dl", @@ -2193,11 +2340,37 @@ mod resume_tests { ); } + /// The dl a restored pane runs is the one that was run, by file name: a path + /// is one herdr refuses and `pixi global update` moves, `aid` (or `dla`) resumes + /// under its own `dl`, and a name no shell can type falls back to the released `dl`. + #[test] + fn a_restored_pane_runs_the_dl_that_was_started() { + let cases = [ + (Some("dl"), "dl"), + (Some("dl-next"), "dl-next"), + (Some("/home/u/.local/bin/dl-next"), "dl-next"), + (Some("aid"), "dl"), + (Some("/home/u/.pixi/bin/aid-next"), "dl-next"), + (Some("dla"), "dl"), + (Some("/home/u/.local/bin/dla-next"), "dl-next"), + (Some("./target/debug/dl"), "dl"), + (None, "dl"), + (Some(""), "dl"), + (Some("/usr/bin/"), "dl"), + (Some("-dl"), "dl"), + (Some("my dl"), "dl"), + (Some("it's"), "dl"), + ]; + for (invoked_as, program) in cases { + assert_eq!(restore_program(invoked_as), program, "{invoked_as:?}"); + } + } + /// A profile is per launch and stored nowhere, so the restored line has to say it. #[test] fn the_profile_this_launch_forwarded_is_named_again() { let argv = claude_resume() - .argv("ws-id", Some("work")) + .argv("dl", "ws-id", Some("work")) .expect("acceptable"); assert_eq!(argv[..5], ["dl", "ws-id", "--claude-profile", "work", "--"]); } @@ -2206,17 +2379,17 @@ mod resume_tests { #[test] fn a_line_herdr_would_refuse_is_not_offered() { let apostrophe = resume_of(words(&["claude", "it's"]), None); - assert_eq!(apostrophe.argv("ws", None), None); + assert_eq!(apostrophe.argv("dl", "ws", None), None); // `dl ws --` is three of the 64. let long = resume_of(words(&vec!["x"; 62]), None); - assert_eq!(long.argv("ws", None).map(|argv| argv.len()), None); + assert_eq!(long.argv("dl", "ws", None).map(|argv| argv.len()), None); let fits = resume_of(words(&vec!["x"; 61]), None); - assert_eq!(fits.argv("ws", None).map(|argv| argv.len()), Some(64)); + assert_eq!(fits.argv("dl", "ws", None).map(|argv| argv.len()), Some(64)); let big = "y".repeat(RESUME_MAX_BYTES); let heavy = resume_of(words(&["claude", &big]), None); - assert_eq!(heavy.argv("ws", None), None); + assert_eq!(heavy.argv("dl", "ws", None), None); assert!(!herdr_accepts(&["/usr/bin/dl".to_owned()])); assert!(!herdr_accepts(&[])); @@ -2228,11 +2401,11 @@ mod resume_tests { let head: usize = ["dl", "ws", "--", "claude"].iter().map(|w| w.len()).sum(); let at_limit = "y".repeat(RESUME_MAX_BYTES - head); let fits = resume_of(words(&["claude", &at_limit]), None); - assert!(fits.argv("ws", None).is_some(), "exactly the limit"); + assert!(fits.argv("dl", "ws", None).is_some(), "exactly the limit"); let over = format!("{at_limit}y"); let heavy = resume_of(words(&["claude", &over]), None); - assert_eq!(heavy.argv("ws", None), None, "one byte over"); + assert_eq!(heavy.argv("dl", "ws", None), None, "one byte over"); } /// A `by_id` line that fits alone can still be pushed over by the id. @@ -2252,27 +2425,28 @@ mod resume_tests { let room = "y".repeat(RESUME_MAX_BYTES - head - UUID); assert!( - with_id(&room).hook_prefix("ws", None).is_some(), + with_id(&room).hook_prefix("dl", "ws", None).is_some(), "the id lands exactly on the limit" ); let crowded = format!("{room}y"); assert!(herdr_accepts(&restore_line( + "dl", "ws", None, words(&["claude", &crowded, "--resume"]).iter() ))); - assert_eq!(with_id(&crowded).hook_prefix("ws", None), None); + assert_eq!(with_id(&crowded).hook_prefix("dl", "ws", None), None); } #[test] fn the_hook_gets_the_line_an_id_completes() { assert_eq!( - claude_resume().hook_prefix("ws-id", None).as_deref(), + claude_resume().hook_prefix("dl", "ws-id", None).as_deref(), Some("dl ws-id -- IS_SANDBOX=1 claude --resume") ); let no_hook = resume_of(words(&["codex", "resume", "--last"]), None); - assert_eq!(no_hook.hook_prefix("ws-id", None), None); + assert_eq!(no_hook.hook_prefix("dl", "ws-id", None), None); } /// The hook splits on spaces, so a word holding one would arrive as two. @@ -2282,9 +2456,9 @@ mod resume_tests { words(&["claude", "--resume", "u"]), Some(words(&["claude", "--name", "a b", "--resume"])), ); - assert_eq!(spaced.hook_prefix("ws", None), None); + assert_eq!(spaced.hook_prefix("dl", "ws", None), None); assert_eq!( - spaced.argv("ws", None).map(|argv| argv.len()), + spaced.argv("dl", "ws", None).map(|argv| argv.len()), Some(6), "the host's own report does not split, so it still goes" ); @@ -2353,6 +2527,27 @@ mod resume_tests { ); } + /// The scan of every pane needs no pane of its own, where the report does. + #[test] + fn a_herdr_is_located_without_a_pane_id_and_falls_back_to_path() { + let host = HostEnv { + pane_id: Some(" ".to_owned()), + ..in_a_pane() + }; + assert_eq!( + Locator::resolve(&host, Some("")), + Some(Locator { + binary: "herdr".to_owned(), + pane_id: None, + session_file: PathBuf::from("/home/u/.config/herdr/sessions/s/session.json"), + }) + ); + assert_eq!( + Locator::resolve(&in_a_pane(), Some("/opt/herdr")).map(|at| (at.binary, at.pane_id)), + Some(("/opt/herdr".to_owned(), Some("w1:p3".to_owned()))) + ); + } + /// The argv herdr 0.9.2 took, in the order it took it: pane first, `--` last. #[test] fn the_report_names_the_argv_and_no_state() { @@ -2411,6 +2606,26 @@ mod resume_tests { assert_eq!(report.saved_in(&session(r#"["dl","other"]"#)), Saved::Other); } + /// herdr numbers panes in Crockford base32: `w3:pAX` is public number 349, + /// as a live herdr's `session.json` showed (`"226": 349`). Ids up to `p9` + /// read the same in base 10, which is how a decimal parse passed the tests above. + #[test] + fn a_pane_id_is_read_as_herdrs_base32_number() { + let json = session(r#"["dl","ws"]"#).replace( + r#""id":"w1","public_pane_numbers":{"1":1,"4":3}"#, + r#""id":"w3","public_pane_numbers":{"1":1,"4":349}"#, + ); + assert_eq!( + saved_argv(&json, "w3:pAX"), + Some(Some(vec!["dl".to_owned(), "ws".to_owned()])) + ); + assert_eq!( + saved_argv(&json, "w3:pax"), + Some(Some(vec!["dl".to_owned(), "ws".to_owned()])) + ); + assert_eq!(saved_argv(&json, "w3:pAY"), Some(None)); + } + #[test] fn a_pane_with_no_argv_lacks_one() { let json = session(r#"["dl","ws"]"#).replace("\"agent_resume\"", "\"other\""); diff --git a/rust/devlaunch-core/src/clients/mod.rs b/rust/devlaunch-core/src/clients/mod.rs index 8f0c5a3f..049663ae 100644 --- a/rust/devlaunch-core/src/clients/mod.rs +++ b/rust/devlaunch-core/src/clients/mod.rs @@ -29,10 +29,13 @@ pub mod git; pub(crate) mod herdr; // Binary surface: `dl` uses the same agent-name reader before it arranges a // Herdr editor split, and the same executable repair before it asks Herdr to do -// so. Only these decisions cross the crate boundary. +// so, and the resume line of each known agent for the notice a recreate gives +// when it ends one it cannot restart. Only these decisions cross the crate +// boundary. pub use herdr::{ AgentResume, agent_named as herdr_agent_named, binary_from_process as herdr_binary_from_process, is_assignment as herdr_is_assignment, + resume_line as herdr_resume_line, resume_lines as herdr_resume_lines, }; // Crate-private for docker's reason: the two signals `dl kill` sends are // the whole of what devlaunch asks of `kill(1)`, and what they came to is diff --git a/rust/devlaunch-core/src/domain/mod.rs b/rust/devlaunch-core/src/domain/mod.rs index 1581cdd4..9f77d032 100644 --- a/rust/devlaunch-core/src/domain/mod.rs +++ b/rust/devlaunch-core/src/domain/mod.rs @@ -6,6 +6,8 @@ pub mod locks; pub mod metadata; // binary surface — not part of the frozen wf API (#251 §7) pub mod model; +/// One `/proc//stat` line, read for the flows that list processes. +pub(crate) mod proc_stat; // binary surface — not part of the frozen wf API (#251 §7) pub mod pull_request; // binary surface — not part of the frozen wf API (#251 §7) diff --git a/rust/devlaunch-core/src/domain/proc_stat.rs b/rust/devlaunch-core/src/domain/proc_stat.rs new file mode 100644 index 00000000..7d526a89 --- /dev/null +++ b/rust/devlaunch-core/src/domain/proc_stat.rs @@ -0,0 +1,86 @@ +//! One line of `/proc//stat`, read the one way every caller shares. +//! +//! Two flows read process tables, on the host and inside containers: the agent +//! lock check (`flows::agent_worktrees`) and the refresh of stale workspaces +//! (`flows::refresh_stale`). The line has one shape, so it has one parser. + +/// The fields dl reads out of one `/proc//stat`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ProcStat { + pub(crate) pid: u32, + /// The command name, as the kernel keeps it: at most 15 bytes, and any + /// bytes at all, spaces and parentheses included. + pub(crate) comm: String, + /// A zombie: exited, and not yet reaped by its parent. + pub(crate) zombie: bool, + pub(crate) ppid: u32, + /// `PF_KTHREAD` is in the flags: a thread of the kernel's own. + pub(crate) kernel_thread: bool, + /// The start time, in clock ticks after boot. + pub(crate) start: u64, +} + +/// `PF_KTHREAD`, in the flags field of `/proc//stat`. Set by the kernel on +/// its own threads, and nothing in user space can set it. +const PF_KTHREAD: u64 = 0x0020_0000; + +/// Read one `/proc//stat` line, or nothing when it is not that shape. +/// +/// The command name sits in parentheses and may hold anything, `) ` included, +/// so the name ends at the *last* `)` and the fields are counted from there. +/// After the name: the state is the 1st field, the parent the 2nd, the flags +/// the 7th (field 9 of the line) and the start time the 20th (field 22). +pub(crate) fn parse(line: &str) -> Option { + let line = line.strip_suffix('\n').unwrap_or(line); + let (head, rest) = line.split_once(" (")?; + let pid = digits(head, 10)?.parse().ok()?; + let close = rest.rfind(')')?; + let after: Vec<&str> = rest[close + 1..].split_whitespace().collect(); + let flags: u64 = digits(after.get(6)?, 20)?.parse().ok()?; + Some(ProcStat { + pid, + comm: rest[..close].to_owned(), + zombie: *after.first()? == "Z", + ppid: digits(after.get(1)?, 10)?.parse().ok()?, + kernel_thread: flags & PF_KTHREAD != 0, + start: digits(after.get(19)?, 20)?.parse().ok()?, + }) +} + +/// `text` when it is all ASCII digits, at least one and at most `at_most`. +pub(crate) fn digits(text: &str, at_most: usize) -> Option<&str> { + (!text.is_empty() && text.len() <= at_most && text.bytes().all(|it| it.is_ascii_digit())) + .then_some(text) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A real line, from a process that named itself `a) b` with `prctl`. + const ODD_NAME: &str = "1234 (a) b) S 1 1234 1234 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 \ + 329153 1000 10 18446744073709551615 0 0 0 0 0 0 0 0 0 0 0 0 17 3 0 \ + 0 0 0 0\n"; + + #[test] + fn a_name_that_holds_a_closing_parenthesis_ends_at_the_last_one() { + assert_eq!( + parse(ODD_NAME), + Some(ProcStat { + pid: 1234, + comm: "a) b".to_owned(), + zombie: false, + ppid: 1, + kernel_thread: false, + start: 329_153, + }) + ); + } + + #[test] + fn a_line_cut_short_or_not_a_stat_line_is_nothing() { + assert_eq!(parse("8621 (short) S 1"), None); + assert_eq!(parse("garbage"), None); + assert_eq!(parse("x (y"), None); + } +} diff --git a/rust/devlaunch-core/src/flows/agent_sessions.rs b/rust/devlaunch-core/src/flows/agent_sessions.rs index 73deae74..63c7d2ee 100644 --- a/rust/devlaunch-core/src/flows/agent_sessions.rs +++ b/rust/devlaunch-core/src/flows/agent_sessions.rs @@ -43,7 +43,7 @@ const ASK_WITHIN: Duration = Duration::from_secs(2); /// The herdr this process can ask, and where it keeps its saved session. #[derive(Clone, Debug, PartialEq, Eq)] -pub struct Manager { +pub struct HerdrPanes { binary: String, /// The pane this process runs in. Nothing types into it: its foreground is /// this `dl`. @@ -51,7 +51,7 @@ pub struct Manager { session_file: PathBuf, } -impl Manager { +impl HerdrPanes { /// The herdr whose pane `host` runs in, or `None` outside one. pub fn from_host(host: &crate::flows::launch::Host) -> Option { Self::resolve(&host.herdr, host.herdr_bin.as_deref()) @@ -59,20 +59,18 @@ impl Manager { /// The herdr whose pane `env` describes, or `None` outside one. /// - /// The socket is required because its directory is where herdr saves the - /// session, which is the one record of how each pane's agent starts again. - /// `binary` falls back to a `PATH` lookup, the rule every other herdr call in - /// dl follows. + /// [`herdr::Locator`] holds the rule. The socket it requires matters here + /// because its directory is where herdr saves the session, which is the one + /// record of how each pane's agent starts again. pub(crate) fn resolve(env: &herdr::HostEnv, binary: Option<&str>) -> Option { - if !crate::flows::provision::provisioning_disabled(env.in_pane.as_deref()) { - return None; - } - let socket = herdr::non_empty(env.socket.as_deref())?; - let session_file = Path::new(&socket).parent()?.join("session.json"); + let herdr::Locator { + binary, + pane_id, + session_file, + } = herdr::Locator::resolve(env, binary)?; Some(Self { - binary: herdr::non_empty(binary) - .unwrap_or_else(|| crate::flows::launch::HERDR_BIN_FALLBACK.to_owned()), - own_pane: herdr::non_empty(env.pane_id.as_deref()), + binary, + own_pane: pane_id, session_file, }) } @@ -95,6 +93,14 @@ pub struct HeldSession { pub line: Vec, } +impl HeldSession { + /// The line as one shell line: what is typed into the pane, or handed to + /// the user to run there. + pub fn shell_line(&self) -> String { + crate::shell::join(self.line.iter().map(String::as_str)) + } +} + /// What [`collect`] found in herdr's panes for one workspace. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct HeldSessions { @@ -130,7 +136,7 @@ pub struct UnresumablePane { /// `agent get` is no reason to drop a line it saved. pub fn collect( runner: &dyn Runner, - manager: &Manager, + manager: &HerdrPanes, workspace_id: &str, read: &dyn Fn(&Path) -> Option, ) -> Option { @@ -156,30 +162,33 @@ pub fn collect( .and_then(|json| herdr::saved_argv(json, &pane.pane_id)) .flatten() .filter(|argv| starts_an_agent(argv, workspace_id)); - let live_agent = match agent_reading(runner, manager, &pane.pane_id) { - herdr::AgentReading::Unanswered => None, - herdr::AgentReading::NoAgent => Some(None), - // Every status, `done` included: herdr's `done` is an agent ready for - // input that nobody has looked at yet, not one that has exited. - herdr::AgentReading::Agent(agent) => Some(Some(agent.agent)), - }; - match (line, live_agent) { - (Some(line), Some(Some(_)) | None) => held.sessions.push(HeldSession { - pane_id: pane.pane_id, - line, - }), - (None, Some(Some(agent))) => held.unresumable.push(UnresumablePane { + // Every status counts as a live agent, `done` included: herdr's `done` is + // an agent ready for input that nobody has looked at yet, not one that + // has exited. + match (line, agent_reading(runner, manager, &pane.pane_id)) { + (Some(line), herdr::AgentReading::Agent(_) | herdr::AgentReading::Unanswered) => { + held.sessions.push(HeldSession { + pane_id: pane.pane_id, + line, + }); + } + (None, herdr::AgentReading::Agent(agent)) => held.unresumable.push(UnresumablePane { pane_id: pane.pane_id, - agent, + agent: agent.agent, }), - (Some(_) | None, _) => {} + (Some(_), herdr::AgentReading::NoAgent) + | (None, herdr::AgentReading::NoAgent | herdr::AgentReading::Unanswered) => {} } } Some(held) } /// What herdr says one pane's agent is doing. -pub fn agent_reading(runner: &dyn Runner, manager: &Manager, pane_id: &str) -> herdr::AgentReading { +pub(crate) fn agent_reading( + runner: &dyn Runner, + manager: &HerdrPanes, + pane_id: &str, +) -> herdr::AgentReading { manager .ask(runner, &herdr::agent_get_argv(pane_id)) .as_deref() @@ -194,13 +203,16 @@ pub fn agent_reading(runner: &dyn Runner, manager: &Manager, pane_id: &str) -> h /// another workspace was saved by an earlier session in the same pane, and would /// start its agent there. fn starts_an_agent(argv: &[String], workspace_id: &str) -> bool { - argv.get(1).map(String::as_str) == Some(workspace_id) - && argv - .iter() - .skip_while(|word| *word != "--") - .skip(1) - .find(|word| !herdr::is_assignment(word)) - .is_some_and(|program| herdr::agent_named(program).is_some()) + argv.get(1).map(String::as_str) == Some(workspace_id) && agent_started_by(argv).is_some() +} + +/// The agent the words after a line's `--` start, by herdr's name for it. +fn agent_started_by(argv: &[String]) -> Option<&'static str> { + argv.iter() + .skip_while(|word| *word != "--") + .skip(1) + .find(|word| !herdr::is_assignment(word)) + .and_then(|program| herdr::agent_named(program)) } /// What became of one held session after the recreate. @@ -235,7 +247,7 @@ pub(crate) const RUN_TICK: Duration = Duration::from_millis(500); /// is handed back for the caller to say. pub fn restart( runner: &dyn Runner, - manager: &Manager, + manager: &HerdrPanes, held: &HeldSessions, wait: &dyn Fn(Duration), ) -> Vec { @@ -243,7 +255,7 @@ pub fn restart( .iter() .map(|session| { let pane_id = session.pane_id.clone(); - let line = crate::shell::join(session.line.iter().map(String::as_str)); + let line = session.shell_line(); if manager.own_pane.as_deref() == Some(pane_id.as_str()) { return Restarted::OwnPane { pane_id, line }; } @@ -261,6 +273,59 @@ pub fn restart( .collect() } +/// Whether herdr kept the line of an agent that [`restart`] typed back. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum LineKept { + Kept, + /// herdr refused the report, or never took it in the time allowed. The next + /// recreate finds no line for the pane, and ends its agent. + NotKept, + /// herdr took the report, and its session file could not be read back to + /// see that it kept the line. Nobody confirmed it, so it is not [`Kept`]. + /// + /// [`Kept`]: LineKept::Kept + Unchecked, +} + +/// Report a typed-back line to herdr as the pane's line again, and check that +/// herdr kept it. +/// +/// The `dl` the line runs is given no line to report: only `aid` hands one over, +/// and dl does not guess another program's flags from a command. Without this, +/// herdr drops the old line once the old agent ends, and the agent can be +/// started again once only. The line is the one herdr saved, so it is already a +/// line herdr accepts. `seq`, `read` and `wait` are as for +/// [`session_manager::report_resume`]; `wait` answers `true` here, because +/// nothing ends the wait early. +pub(crate) fn report_again( + runner: &dyn Runner, + manager: &HerdrPanes, + session: &HeldSession, + seq: &dyn Fn() -> u128, + read: &dyn Fn(&Path) -> Option, + wait: &dyn Fn(Duration) -> bool, +) -> LineKept { + let Some(agent) = agent_started_by(&session.line) else { + return LineKept::NotKept; + }; + let report = herdr::ResumeReport::for_pane( + &manager.binary, + &session.pane_id, + &manager.session_file, + agent, + session.line.clone(), + ); + match session_manager::report_resume(runner, &report, seq, read, wait) { + session_manager::ResumeReported::Saved | session_manager::ResumeReported::Superseded => { + LineKept::Kept + } + session_manager::ResumeReported::Taken => LineKept::Unchecked, + session_manager::ResumeReported::Dropped | session_manager::ResumeReported::Stopped => { + LineKept::NotKept + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -279,8 +344,8 @@ mod tests { } } - fn manager() -> Manager { - Manager::resolve(&env("w1:p9"), Some(HERDR)).expect("a herdr pane") + fn manager() -> HerdrPanes { + HerdrPanes::resolve(&env("w1:p9"), Some(HERDR)).expect("a herdr pane") } fn pane_list(panes: &[&str]) -> String { @@ -883,9 +948,112 @@ mod tests { fn outside_a_herdr_pane_there_is_no_manager_to_ask() { let mut outside = env("w1:p9"); outside.in_pane = None; - assert_eq!(Manager::resolve(&outside, Some(HERDR)), None); + assert_eq!(HerdrPanes::resolve(&outside, Some(HERDR)), None); let mut no_socket = env("w1:p9"); no_socket.socket = None; - assert_eq!(Manager::resolve(&no_socket, Some(HERDR)), None); + assert_eq!(HerdrPanes::resolve(&no_socket, Some(HERDR)), None); + } + + fn one_held() -> HeldSession { + HeldSession { + pane_id: "w1:p1".to_owned(), + line: words(CLAUDE_LINE), + } + } + + /// The `dl` a typed line runs reports no line of its own, so the recreate + /// reports the one it typed, as the pane's line, and reads it back. + #[test] + fn a_typed_line_is_reported_to_herdr_again_as_the_panes_line() { + let runner = ScriptedRunner::new().with_script([HERDR], Response::ok()); + + let kept = report_again( + &runner, + &manager(), + &one_held(), + &|| 7, + &|path| { + assert_eq!(path, Path::new("/run/herdr/session.json")); + Some(saved(&[Some(CLAUDE_LINE)])) + }, + &|_| panic!("herdr kept it at once"), + ); + + assert_eq!(kept, LineKept::Kept); + let mut expected: Vec = [ + "pane", + "report-agent-session", + "w1:p1", + "--source", + "devlaunch:claude", + "--agent", + "claude", + "--seq", + "7", + "--", + ] + .map(str::to_owned) + .to_vec(); + expected.extend(words(CLAUDE_LINE)); + assert_eq!(runner.args_to(HERDR), vec![expected]); + } + + /// herdr refuses a report until it sees the new agent in the pane, and the + /// typed `dl` takes seconds to start one. + #[test] + fn a_report_sent_before_the_new_agent_is_seen_is_sent_again() { + let runner = ScriptedRunner::new().with_script( + [HERDR], + Response::failed( + 1, + r#"{"error":{"code":"resume_not_accepted","message":"report its state first"}}"#, + ), + ); + let waits = std::cell::Cell::new(0); + + let kept = report_again( + &runner, + &manager(), + &one_held(), + &|| 1, + &|_| Some(saved(&[Some(CLAUDE_LINE)])), + &|_| { + waits.set(waits.get() + 1); + if waits.get() == 2 { + runner.clear_scripts().script([HERDR], Response::ok()); + } + true + }, + ); + + assert_eq!(kept, LineKept::Kept); + assert_eq!(runner.args_to(HERDR).len(), 3); + } + + #[test] + fn a_report_herdr_refuses_for_good_is_a_line_not_kept() { + let runner = ScriptedRunner::new().with_script( + [HERDR], + Response::failed(1, r#"{"error":{"code":"pane_not_found"}}"#), + ); + + let kept = report_again(&runner, &manager(), &one_held(), &|| 1, &|_| None, &|_| { + panic!("a refusal for good is not asked again") + }); + + assert_eq!(kept, LineKept::NotKept); + } + + /// herdr takes the report, and its session file cannot be read back. Nothing + /// confirmed the line, so it does not read as kept. + #[test] + fn a_report_whose_session_file_cannot_be_read_back_is_unchecked() { + let runner = ScriptedRunner::new().with_script([HERDR], Response::ok()); + + let kept = report_again(&runner, &manager(), &one_held(), &|| 1, &|_| None, &|_| { + panic!("an unreadable file is not read again") + }); + + assert_eq!(kept, LineKept::Unchecked); } } diff --git a/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock.rs b/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock.rs index 29746152..23798c98 100644 --- a/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock.rs +++ b/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock.rs @@ -83,6 +83,7 @@ use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; use crate::clients::docker; +use crate::domain::proc_stat::{self, ProcStat, digits}; use crate::domain::workspace_state::NonEmpty; use crate::runner::Runner; @@ -117,11 +118,6 @@ pub(crate) fn parse(reason: &str) -> Option { Some(AgentLock { pid, start }) } -fn digits(text: &str, at_most: usize) -> Option<&str> { - (!text.is_empty() && text.len() <= at_most && text.bytes().all(|it| it.is_ascii_digit())) - .then_some(text) -} - /// What became of a lock's agent. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum Owner { @@ -361,6 +357,7 @@ impl<'r> Owners<'r> { &container.id, &["sh", "-c", READ_THE_TABLE], ) + .map_err(|why| why.to_string()) .and_then(|out| container_table(&out)) { Ok(table) => decide(&table, lock), @@ -647,10 +644,6 @@ fn the_whole_machine(table: &[Process]) -> bool { table.iter().any(|process| process.kernel_thread) } -/// `PF_KTHREAD`, in the flags field of `/proc//stat`. Set by the kernel on -/// its own threads, and nothing in user space can set it. -const PF_KTHREAD: u64 = 0x0020_0000; - /// The pid, in the table's own namespace, of the process that matched. fn running_as(table: &[Process], lock: &AgentLock) -> Option { table @@ -663,33 +656,6 @@ fn running_as(table: &[Process], lock: &AgentLock) -> Option { .map(|process| process.pid) } -/// The fields this module reads out of one `/proc//stat`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct StatFields { - pid: u32, - kernel_thread: bool, - start: u64, -} - -/// The pid, whether it is a kernel thread, and the start time out of one -/// `/proc//stat`. -/// -/// The command name sits in parentheses and may hold anything, spaces and -/// parentheses included, so the fields are counted from the *last* `)`. The -/// flags are field 9, the 7th after the name, and the start time field 22, the -/// 20th. -fn stat_fields(stat: &str) -> Option { - let (head, _) = stat.split_once(" (")?; - let pid = digits(head, 10)?.parse().ok()?; - let after: Vec<&str> = stat[stat.rfind(')')? + 1..].split_whitespace().collect(); - let flags: u64 = digits(after.get(6)?, 20)?.parse().ok()?; - Some(StatFields { - pid, - kernel_thread: flags & PF_KTHREAD != 0, - start: digits(after.get(19)?, 20)?.parse().ok()?, - }) -} - /// The pids out of a `NSpid:` line of `/proc//status`. fn nspid_entries(line: &str) -> Option> { let rest = line.strip_prefix("NSpid:")?; @@ -738,11 +704,12 @@ fn table_of( Err(error) if vanished(&error) => continue, Err(error) => return Err(format!("could not read /proc/{name}/stat: {error}")), }; - let StatFields { + let ProcStat { pid, kernel_thread, start, - } = stat_fields(&stat).ok_or_else(|| format!("could not read /proc/{name}/stat"))?; + .. + } = proc_stat::parse(&stat).ok_or_else(|| format!("could not read /proc/{name}/stat"))?; let nspid = std::fs::read_to_string(entry.join("status")) .ok() .and_then(|status| status.lines().find_map(nspid_entries)); @@ -778,11 +745,12 @@ fn container_table(out: &str) -> Result, String> { .ok_or("the listing stopped before its end")?; let mut table = Vec::new(); for line in stats.lines() { - let StatFields { + let ProcStat { pid, kernel_thread, start, - } = stat_fields(line).ok_or_else(|| format!("a stat line did not parse: {line}"))?; + .. + } = proc_stat::parse(line).ok_or_else(|| format!("a stat line did not parse: {line}"))?; table.push(Process { pid, kernel_thread, diff --git a/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock/tests.rs b/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock/tests.rs index 24a414c3..8acec422 100644 --- a/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock/tests.rs +++ b/rust/devlaunch-core/src/flows/agent_worktrees/agent_lock/tests.rs @@ -137,20 +137,6 @@ fn a_table_that_does_not_list_pid_one_proves_nothing() { assert!(matches!(decide(&[], &incident()), InOne::CouldNotTell(_))); } -#[test] -fn a_stat_line_is_read_from_its_last_parenthesis() { - assert_eq!( - stat_fields(&stat(8621, 329_153)), - Some(StatFields { - pid: 8621, - kernel_thread: false, - start: 329_153 - }) - ); - assert_eq!(stat_fields("8621 (short) S 1"), None); - assert_eq!(stat_fields("garbage"), None); -} - #[test] fn the_containers_listing_is_read_whole_or_not_at_all() { let out = format!( @@ -803,11 +789,11 @@ fn a_kernel_thread_is_what_marks_the_whole_machine_and_a_parent_of_zero_is_not() // Inside a container, every process `docker exec` started reads as parent // 0, so a parent of 0 says nothing about which namespace this is. assert_eq!( - stat_fields(&stat_with_flags(2, KTHREADD_FLAGS, 0)).map(|it| it.kernel_thread), + proc_stat::parse(&stat_with_flags(2, KTHREADD_FLAGS, 0)).map(|it| it.kernel_thread), Some(true) ); assert_eq!( - stat_fields(&stat_with_flags(90_718, 4_194_560, 0)).map(|it| it.kernel_thread), + proc_stat::parse(&stat_with_flags(90_718, 4_194_560, 0)).map(|it| it.kernel_thread), Some(false) ); } diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 7f661ec2..04d4e915 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -70,6 +70,7 @@ use crate::clients::devpod::{ ProviderRegistration, }; use crate::clients::devpod_home::{CreateRecord, DevpodHome, create_record}; +use crate::clients::docker; use crate::clients::gh::{self, GhEvent, StagedToken, Token, TokenLookup}; use crate::clients::herdr; use crate::clients::ssh; @@ -222,6 +223,9 @@ pub struct Host { pub(crate) herdr_tab_id: Option, /// `HERDR_BIN_PATH`: which herdr to ask, falling back to `PATH`. pub(crate) herdr_bin: Option, + /// argv\[0\] as this process was started: the `dl` a resume line names + /// ([`herdr::restore_program`]). `None` reads as the released `dl`. + pub(crate) invoked_as: Option, /// How the agent this launch starts is started again after herdr restarts, /// when the caller knows. Not read from the environment: `aid` knows it because /// it wrote the agent's line, and hands it over with [`Self::with_agent_resume`]. @@ -297,6 +301,7 @@ impl Host { no_title: crate::osext::env_str(TITLE_DISABLE_VAR), herdr_tab_id: crate::osext::env_str(HERDR_TAB_VAR), herdr_bin: herdr::binary_from_process(), + invoked_as: std::env::args().next(), agent_resume: None, stdin_tty: is_a_terminal(libc::STDIN_FILENO), stdout_tty: is_a_terminal(libc::STDOUT_FILENO), @@ -433,6 +438,19 @@ fn switched_on(value: Option<&str>) -> bool { // the notices // =========================================================================== +/// A stale workspace whose image devpod built, and devpod's tag for that image +/// could not be removed: a recreate would build the container from the older +/// image again. [`LaunchNotice::DerivedImageKept`] says it when the recreate +/// goes ahead; [`LaunchRefusal::DerivedImageKept`] when it does not run. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct DerivedImageKept { + pub workspace_id: String, + /// devpod's tag for the image. + pub image: String, + /// Why the tag could not be removed. + pub why: String, +} + /// Something the launch did that the `dl` binary may want to report. /// /// One vocabulary for the whole flow, because a single launch produces notices @@ -696,6 +714,19 @@ pub enum LaunchNotice { }, /// The agent's line was typed back into its pane. AgentSessionRestarted { pane_id: String }, + /// The agent was started again, and herdr did not keep its line, so the next + /// recreate ends it. + AgentSessionLineNotKept { pane_id: String }, + /// The agent was started again, and herdr took its line, but dl could not + /// read herdr's session file back to check that herdr kept it. + AgentSessionLineUnchecked { pane_id: String }, + /// The workspace is stale and its image is one devpod built on the older + /// reference, so the recreate removed devpod's tag for it first: devpod + /// reuses a tag it finds, and builds again only when it is gone. + DerivedImageRemoved { workspace_id: String, image: String }, + /// The tag could not be removed, so the recreate builds the container from + /// the older image again and the workspace stays stale. + DerivedImageKept(DerivedImageKept), /// The agent's line could not be typed into its pane; `line` is what to run /// there by hand. AgentSessionNotRestarted { pane_id: String, line: String }, @@ -1414,7 +1445,7 @@ pub(crate) fn serialize_launch( } } -fn lock_reason(error: &LockError) -> String { +pub(crate) fn lock_reason(error: &LockError) -> String { match error { LockError::CreateParent { failure, .. } | LockError::Open { failure, .. } @@ -2867,8 +2898,13 @@ pub(crate) fn workspace_ssh( // session turns out to take. // With the line the container hook completes when the agent reports a new // session id, so a restart after `/clear` reopens the session in the pane. + let program = herdr::restore_program(session.host.invoked_as.as_deref()); let hook_prefix = session.host.agent_resume.as_ref().and_then(|resume| { - resume.hook_prefix(workspace_id, session.host.claude.profile.as_deref()) + resume.hook_prefix( + &program, + workspace_id, + session.host.claude.profile.as_deref(), + ) }); let relay = herdr::Reporting::resolve(&session.host.herdr) .map(|reporting| reporting.with_resume_prefix(hook_prefix)) @@ -2890,7 +2926,11 @@ pub(crate) fn workspace_ssh( &session.host.herdr, session.host.herdr_bin.as_deref(), agent, - resume.argv(workspace_id, session.host.claude.profile.as_deref()), + resume.argv( + &program, + workspace_id, + session.host.claude.profile.as_deref(), + ), ) }); let session_outcome = beside_resume_report(session.runner, resume.as_ref(), || { @@ -2934,6 +2974,68 @@ pub(crate) fn workspace_ssh( session_outcome } +/// Report each line typed back into a pane as that pane's line again, and answer +/// the panes whose line nobody saw herdr keep, each with what became of it. +/// +/// One thread per pane, because each one waits up to seconds for herdr to see +/// its new agent, and the waits overlap. +fn report_typed_lines_again( + runner: &dyn Runner, + manager: &agent_sessions::HerdrPanes, + held: &agent_sessions::HeldSessions, + restarted: &[agent_sessions::Restarted], +) -> Vec<(String, agent_sessions::LineKept)> { + unconfirmed_lines(held, restarted, &|session| { + agent_sessions::report_again( + runner, + manager, + session, + &report_seq, + &read_session_file, + &|pause| { + std::thread::sleep(pause); + true + }, + ) + }) +} + +/// The panes among the typed ones whose line `report` did not see kept, each +/// with what `report` came to, one thread per pane. +/// +/// A report that panics confirmed nothing, so its pane counts as not kept: the +/// user is told, rather than left to find the agent ended at the next recreate. +fn unconfirmed_lines( + held: &agent_sessions::HeldSessions, + restarted: &[agent_sessions::Restarted], + report: &(dyn Fn(&agent_sessions::HeldSession) -> agent_sessions::LineKept + Sync), +) -> Vec<(String, agent_sessions::LineKept)> { + let typed: Vec<&agent_sessions::HeldSession> = held + .sessions + .iter() + .filter(|session| { + restarted.iter().any(|outcome| { + matches!(outcome, agent_sessions::Restarted::Typed { pane_id } if *pane_id == session.pane_id) + }) + }) + .collect(); + std::thread::scope(|scope| { + let reports: Vec<_> = typed + .iter() + .map(|session| (session, scope.spawn(move || report(session)))) + .collect(); + reports + .into_iter() + .filter_map(|(session, report)| { + match report.join().unwrap_or(agent_sessions::LineKept::NotKept) { + agent_sessions::LineKept::Kept => None, + unconfirmed => Some((session.pane_id.clone(), unconfirmed)), + } + }) + .collect() + }) +} + /// Run `session` with the resume argv reported beside it, on a thread of its own. /// /// The report is retried for seconds and the session lasts hours, so the report @@ -2954,12 +3056,8 @@ fn beside_resume_report( session_manager::report_resume( runner, report, - &|| { - SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .map_or(0, |since| since.as_nanos()) - }, - &|path| std::fs::read_to_string(path).ok(), + &report_seq, + &read_session_file, &|pause| stop.wait(pause), ) }); @@ -2969,6 +3067,19 @@ fn beside_resume_report( }) } +/// The number on a report to herdr: the wall clock in nanoseconds, so a later +/// report always outnumbers an earlier one, and herdr drops the older. +fn report_seq() -> u128 { + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map_or(0, |since| since.as_nanos()) +} + +/// Read herdr's session file, or nothing when it cannot be read. +fn read_session_file(path: &std::path::Path) -> Option { + std::fs::read_to_string(path).ok() +} + /// A flag one thread raises and another sleeps on. #[derive(Default)] struct Stop { @@ -4620,6 +4731,10 @@ pub enum LaunchVerb { Code, /// `dl recreate`: rebuild the container, then attach. Recreate, + /// `dl --refresh-stale`: rebuild the container and start its held agents + /// again, as `recreate` does, and attach nothing. It runs unattended, after a + /// daily pull, where there is nobody to hand a session to. + RecreateUnattended, /// `dl reset`: clean slate, then attach. Reset, /// `dl restart`: stop and start without rebuilding, then attach. @@ -4632,7 +4747,7 @@ impl LaunchVerb { /// What this verb asks devpod to rebuild. fn rebuild(&self) -> Rebuild { match self { - Self::Recreate => Rebuild::Recreate, + Self::Recreate | Self::RecreateUnattended => Rebuild::Recreate, Self::Reset => Rebuild::Reset, Self::Attach { .. } | Self::Up | Self::Code | Self::Restart | Self::Dotfiles => { Rebuild::Reuse @@ -4647,6 +4762,7 @@ impl LaunchVerb { Self::Attach { .. } | Self::Up | Self::Recreate + | Self::RecreateUnattended | Self::Reset | Self::Restart | Self::Dotfiles => Ide::NoIde, @@ -4658,7 +4774,7 @@ impl LaunchVerb { match self { Self::Attach { command } => command.as_ref(), Self::Recreate | Self::Reset | Self::Restart => None, - Self::Up | Self::Code | Self::Dotfiles => None, + Self::Up | Self::Code | Self::Dotfiles | Self::RecreateUnattended => None, } } @@ -4672,6 +4788,18 @@ impl LaunchVerb { matches!(self, Self::Up | Self::Code) } + /// Whether this verb rebuilds the container in place: `recreate`, and the + /// unattended recreate `dl --refresh-stale` runs. + fn is_recreate(&self) -> bool { + matches!(self, Self::Recreate | Self::RecreateUnattended) + } + + /// Whether nobody is there to read a notice or take a session: the recreate + /// `dl --refresh-stale` runs after a daily pull. + fn is_unattended(&self) -> bool { + matches!(self, Self::RecreateUnattended) + } + /// Whether this verb hands the user a session at the end. fn attaches(&self) -> bool { matches!( @@ -4728,6 +4856,10 @@ pub enum LaunchRefusal { recorded_repo: String, recorded_branch: String, }, + /// An unattended recreate of a stale workspace whose image devpod built, and + /// devpod's tag for it could not be removed. The recreate would build the + /// container from the older image again, so it does not run. + DerivedImageKept(DerivedImageKept), /// A bare `owner/repo` whose default branch could not be named. BranchNotNamed { owner: String, @@ -5059,9 +5191,10 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { LaunchVerb::Up => self.run_up_verb(&placement, devcontainer), LaunchVerb::Restart => self.run_restart(verb, devcontainer, &placement), LaunchVerb::Attach { .. } => self.run_attach(raw_spec, verb, devcontainer, &placement), - LaunchVerb::Code | LaunchVerb::Recreate | LaunchVerb::Reset => { - self.run_rebuild(verb, devcontainer, &placement) - } + LaunchVerb::Code + | LaunchVerb::Recreate + | LaunchVerb::RecreateUnattended + | LaunchVerb::Reset => self.run_rebuild(verb, devcontainer, &placement), } } @@ -5127,17 +5260,22 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { session } - /// `dl code`, `recreate`, `reset`. + /// `dl code`, `recreate`, `reset`, and the unattended recreate. fn run_rebuild( &mut self, verb: &LaunchVerb, devcontainer: Option<&DevcontainerPath>, placement: &Placement, ) -> Result { + if verb.is_recreate() + && let Some(refused) = self.drop_derived_image(placement.workspace_id(), verb) + { + return Ok(Launched::Refused(refused)); + } // Only a recreate: `code` keeps the container when nothing changed, and a // `reset` drops the volumes, so neither is the case this is for. - let held = if matches!(verb, LaunchVerb::Recreate) && placement.is_running() { - self.hold_agent_sessions(placement.workspace_id()) + let held = if verb.is_recreate() && placement.is_running() { + self.hold_agent_sessions(placement.workspace_id(), verb) } else { None }; @@ -5148,7 +5286,11 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { // is a devpod that ran and may have removed the container before failing. let untouched = matches!(up, Err(LaunchAborted::DevpodNotRun(NotRun::Blocked(_)))); if let Some((manager, held)) = held.filter(|_| !untouched) { - self.restart_agent_sessions(&manager, &held, matches!(up, Ok(None))); + if matches!(up, Ok(None)) { + self.restart_agent_sessions(&manager, &held); + } else { + self.hand_back_agent_sessions(&held); + } } if let Some(refused) = up? { return Ok(Launched::Refused(refused)); @@ -5162,6 +5304,53 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { session } + /// Remove the tag devpod built a stale workspace's image under, so that the + /// recreate builds it again from the newer reference (devlaunch#673). + /// + /// devpod names that tag by a hash of the config, not of the reference, and + /// reuses a tag it finds: without this a recreate leaves the workspace on the + /// older image. Nothing to do for a workspace that is current, or whose + /// container was made from the reference itself. When the tag stays, a + /// recreate the user asked for goes ahead with a notice; an unattended one is + /// refused, because it would recreate the workspace on every run and never + /// make it current. + fn drop_derived_image( + &mut self, + workspace_id: &str, + verb: &LaunchVerb, + ) -> Option { + let stale = crate::flows::stale_images::stale_images( + self.context.runner(), + self.host.devpod_home.as_ref(), + [workspace_id], + ); + let image = stale.of(workspace_id)?.derived_image()?.to_owned(); + let why = match docker::untag_image(self.context.runner(), &image) { + docker::Answer::Ran { exit, .. } if exit.is_success() => { + self.notices.say(LaunchNotice::DerivedImageRemoved { + workspace_id: workspace_id.to_owned(), + image, + }); + return None; + } + docker::Answer::Ran { stderr, .. } => stderr.trim().to_owned(), + docker::Answer::NotInstalled => "docker was not found".to_owned(), + docker::Answer::NotStarted(failure) => { + format!("docker did not start ({:?})", failure.kind) + } + }; + let kept = DerivedImageKept { + workspace_id: workspace_id.to_owned(), + image, + why, + }; + if verb.is_unattended() { + return Some(LaunchRefusal::DerivedImageKept(kept)); + } + self.notices.say(LaunchNotice::DerivedImageKept(kept)); + None + } + /// Find the agents a recreate of `workspace_id` is about to end, and say what /// will become of each (devlaunch#673). /// @@ -5169,18 +5358,24 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { fn hold_agent_sessions( &mut self, workspace_id: &str, - ) -> Option<(agent_sessions::Manager, agent_sessions::HeldSessions)> { + verb: &LaunchVerb, + ) -> Option<(agent_sessions::HerdrPanes, agent_sessions::HeldSessions)> { let workspace_id = workspace_id.to_owned(); - let Some(manager) = agent_sessions::Manager::from_host(self.host) else { - self.notices - .say(LaunchNotice::AgentSessionsUnseen { workspace_id }); + let Some(manager) = agent_sessions::HerdrPanes::from_host(self.host) else { + // Outside herdr the unattended recreate warns of nothing: `dl + // --refresh-stale` recreates there only a container it saw no Claude in. + if !verb.is_unattended() { + self.notices + .say(LaunchNotice::AgentSessionsUnseen { workspace_id }); + } return None; }; - let Some(held) = - agent_sessions::collect(self.context.runner(), &manager, &workspace_id, &|path| { - std::fs::read_to_string(path).ok() - }) - else { + let Some(held) = agent_sessions::collect( + self.context.runner(), + &manager, + &workspace_id, + &read_session_file, + ) else { self.notices .say(LaunchNotice::AgentSessionsUnanswered { workspace_id }); return None; @@ -5205,29 +5400,37 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { Some((manager, held)) } - /// Start each held agent again in its pane, or, when the recreate's `up` did - /// not finish, hand each one's line to the user to run by hand. + /// When the recreate's `up` did not finish, hand each held agent's line to + /// the user to run by hand. + fn hand_back_agent_sessions(&mut self, held: &agent_sessions::HeldSessions) { + for session in &held.sessions { + self.notices.say(LaunchNotice::AgentSessionNotRestarted { + pane_id: session.pane_id.clone(), + line: session.shell_line(), + }); + } + } + + /// Start each held agent again in its pane, after the recreate's `up` finished. fn restart_agent_sessions( &mut self, - manager: &agent_sessions::Manager, + manager: &agent_sessions::HerdrPanes, held: &agent_sessions::HeldSessions, - up: bool, ) { - if !up { - for session in &held.sessions { - self.notices.say(LaunchNotice::AgentSessionNotRestarted { - pane_id: session.pane_id.clone(), - line: crate::shell::join(session.line.iter().map(String::as_str)), - }); - } - return; - } let restarted = agent_sessions::restart(self.context.runner(), manager, held, &std::thread::sleep); + let unconfirmed = + report_typed_lines_again(self.context.runner(), manager, held, &restarted); for outcome in restarted { self.notices.say(match outcome { agent_sessions::Restarted::Typed { pane_id } => { - LaunchNotice::AgentSessionRestarted { pane_id } + match unconfirmed.iter().find(|(pane, _)| *pane == pane_id) { + Some((_, agent_sessions::LineKept::Unchecked)) => { + LaunchNotice::AgentSessionLineUnchecked { pane_id } + } + Some(_) => LaunchNotice::AgentSessionLineNotKept { pane_id }, + None => LaunchNotice::AgentSessionRestarted { pane_id }, + } } agent_sessions::Restarted::NotTyped { pane_id, line } => { LaunchNotice::AgentSessionNotRestarted { pane_id, line } @@ -5810,6 +6013,47 @@ mod tests { self } + /// devpod's record for a workspace whose container runs an image devpod + /// built, `proj-8260d:devpod-9409`, on an older `ghcr.io/o/img:latest`, + /// and docker's answers that make it stale. + fn with_stale_derived_image(self, workspace_id: &str) -> Self { + let home = self.devpod_home(); + self.write_record(workspace_id); + std::fs::write( + home.result("default", workspace_id), + serde_json::json!({ + "ContainerDetails": { + "ID": "c1", + "Config": { "Image": "proj-8260d:devpod-9409" }, + }, + "MergedConfig": { "image": "ghcr.io/o/img:latest" }, + }) + .to_string(), + ) + .expect("a create result"); + let format = |kind: &str, format: &str| { + ["docker", "inspect", "--type", kind, "--format", format].map(str::to_owned) + }; + self.runner.script( + format("container", "{{.Id}} {{.Image}}"), + Response::stdout("c1 sha256:derived\n"), + ); + let image = format("image", "{{.Id}} {{.Created}} {{json .RootFS.Layers}}"); + let mut derived = image.to_vec(); + derived.push("sha256:derived".to_owned()); + self.runner.script( + derived, + Response::stdout("sha256:derived 2026-10-01T00:00:00Z [\"l1\",\"f1\"]\n"), + ); + let mut reference = image.to_vec(); + reference.push("ghcr.io/o/img:latest".to_owned()); + self.runner.script( + reference, + Response::stdout("sha256:base2 2026-10-05T00:00:00Z [\"l2\"]\n"), + ); + self + } + /// devpod's record for a workspace whose create *aborted*: the record, and /// no result beside it. A `postCreateCommand` that exits non-zero leaves /// exactly this, with the container still up. @@ -9618,6 +9862,48 @@ mod tests { ); } + /// A dev build resumes under itself: the line names the `dl` this process was + /// started as, by file name, so `dl-next` does not come back as the released `dl` + /// (devlaunch#677). Both lines: the one reported now and the hook's prefix. + #[test] + fn a_resume_line_names_the_dl_that_was_run() { + let mut scene = in_a_pane_resuming( + Scene::new().on_a_terminal(&["myws"]).with_running("myws"), + &["claude", "--resume", "u-1"], + ); + scene.host.invoked_as = Some("/home/u/.local/bin/dl-next".to_owned()); + let _ = a_session(&scene, Some(&RemoteCommand::argv(&["claude"]))); + let reports = resume_reports(&scene); + let report = reports.first().expect("a resume report"); + let argv_at = report + .iter() + .position(|word| word == "--") + .expect("an argv") + + 1; + assert_eq!( + report[argv_at..], + ["dl-next", "myws", "--", "claude", "--resume", "u-1"] + ); + + let line = ["claude", "--resume", "u-1"]; + let by_id: &[&str] = &["claude", "--resume"]; + let mut scene = consenting_and_resuming( + Scene::new().on_a_terminal(&["myws"]).with_running("myws"), + &line, + Some(by_id), + ); + scene.host.invoked_as = Some("aid-next".to_owned()); + let _ = a_session(&scene, Some(&RemoteCommand::argv(&["claude"]))); + assert_eq!( + coordinated_ssh(&scene) + .env + .entries + .get(herdr::RESUME_PREFIX_VAR) + .map(String::as_str), + Some("dl-next myws -- claude --resume") + ); + } + /// Nothing to resume without a command that is an agent, or outside a pane. #[test] fn a_launch_that_starts_no_agent_or_is_in_no_pane_reports_nothing() { @@ -13717,6 +14003,9 @@ mod tests { ), ); scene.runner.script([HERDR, "pane", "run"], Response::ok()); + scene + .runner + .script([HERDR, "pane", "report-agent-session"], Response::ok()); scene.runner.script([HERDR], Response::exited(1)); line.into_iter().map(str::to_owned).collect() } @@ -13775,6 +14064,28 @@ mod tests { crate::shell::join(line.iter().map(String::as_str)) ] ); + // The typed `dl` reports no line, so the recreate reports the one it + // typed: without it the next recreate finds none and ends the agent. + let reported = at(&|argv| { + argv.get(1..4) + == Some(&[ + "pane".to_owned(), + "report-agent-session".to_owned(), + "w1:p1".to_owned(), + ]) + }); + assert!( + typed < reported, + "the line was reported before it was typed" + ); + assert_eq!(argvs[reported][11..], line[..]); + assert!( + !parts.said.contains(&LaunchNotice::AgentSessionLineNotKept { + pane_id: "w1:p1".to_owned(), + }), + "{:?}", + parts.said + ); assert!( parts.said.contains(&LaunchNotice::AgentSessionsHeld { workspace_id: "myws".to_owned(), @@ -13792,6 +14103,89 @@ mod tests { ); } + /// The thread that reports a typed line panics, so nothing confirmed the + /// line. The pane counts as not kept, the same as a refused report. + #[test] + fn a_report_that_panics_leaves_its_line_not_kept() { + let held = agent_sessions::HeldSessions { + sessions: vec![ + agent_sessions::HeldSession { + pane_id: "w3:pAX".to_owned(), + line: vec!["dl".to_owned()], + }, + agent_sessions::HeldSession { + pane_id: "w3:pB2".to_owned(), + line: vec!["dl".to_owned()], + }, + ], + unresumable: Vec::new(), + }; + let restarted = [ + agent_sessions::Restarted::Typed { + pane_id: "w3:pAX".to_owned(), + }, + agent_sessions::Restarted::Typed { + pane_id: "w3:pB2".to_owned(), + }, + ]; + + let unconfirmed = unconfirmed_lines(&held, &restarted, &|session| { + assert!(session.pane_id != "w3:pAX", "the report broke"); + agent_sessions::LineKept::Kept + }); + + assert_eq!( + unconfirmed, + [("w3:pAX".to_owned(), agent_sessions::LineKept::NotKept)] + ); + } + + /// herdr refuses the report of the typed line, so the next recreate would + /// find no line for the pane. The recreate says so, rather than that all went + /// well. + #[test] + fn a_recreate_says_when_herdr_did_not_keep_a_restarted_agents_line() { + let mut scene = Scene::new().with_running("myws"); + scene.runner.script( + ["/opt/herdr/bin/herdr", "pane", "report-agent-session"], + Response::failed(1, r#"{"error":{"code":"pane_not_found"}}"#), + ); + in_herdr_beside_claude(&mut scene); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch + .run("myws", &LaunchVerb::Recreate, None) + .expect("a launch"); + } + + assert!( + parts.said.contains(&LaunchNotice::AgentSessionLineNotKept { + pane_id: "w1:p1".to_owned(), + }), + "{:?}", + parts.said + ); + assert!( + !parts.said.contains(&LaunchNotice::AgentSessionRestarted { + pane_id: "w1:p1".to_owned(), + }), + "{:?}", + parts.said + ); + } + /// Outside herdr there are no panes to read, so the recreate says it ends the /// agents and goes ahead. #[test] @@ -13879,6 +14273,219 @@ mod tests { ); } + /// `dl --refresh-stale` runs unattended: the recreate starts the agents + /// again in their panes, and hands no session to anybody. + #[test] + fn an_unattended_recreate_starts_the_agents_again_and_attaches_nothing() { + let mut scene = Scene::new().with_running("myws"); + in_herdr_beside_claude(&mut scene); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("myws", &LaunchVerb::RecreateUnattended, None) + }; + + assert_eq!(launched, Ok(Launched::Ready)); + let argvs = scene.runner.argvs(); + let up = argvs + .iter() + .find(|argv| argv.get(1).map(String::as_str) == Some("up")) + .expect("an up"); + assert!(up.contains(&"--recreate".to_owned()), "{up:?}"); + assert!( + argvs + .iter() + .any(|argv| argv.get(1..3) == Some(&["pane".to_owned(), "run".to_owned()])), + "the agent was not started again: {argvs:?}" + ); + assert!( + !argvs.iter().any(|argv| { + argv.first().map(String::as_str) == Some("devpod") + && argv.get(1).map(String::as_str) == Some("ssh") + }), + "a session was opened: {argvs:?}" + ); + assert!( + parts.said.contains(&LaunchNotice::AgentSessionRestarted { + pane_id: "w1:p1".to_owned(), + }), + "{:?}", + parts.said + ); + } + + fn recreate( + scene: &Scene, + verb: &LaunchVerb, + ) -> (Result, Vec) { + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("myws", verb, None) + }; + (launched, parts.said) + } + + const UNTAG: [&str; 5] = ["docker", "image", "rm", "--force", "proj-8260d:devpod-9409"]; + + /// devpod reuses the tag it built a workspace's image under, so a recreate + /// of a stale workspace removes that tag first, and devpod builds the image + /// again from the newer reference. + #[test] + fn a_recreate_of_a_stale_derived_image_removes_devpods_tag_before_the_up() { + let scene = Scene::new() + .with_running("myws") + .with_stale_derived_image("myws"); + scene.runner.script(UNTAG, Response::ok()); + + let (launched, said) = recreate(&scene, &LaunchVerb::RecreateUnattended); + + assert_eq!(launched, Ok(Launched::Ready)); + let argvs = scene.runner.argvs(); + let at = |wanted: &dyn Fn(&[String]) -> bool| { + argvs + .iter() + .position(|argv| wanted(argv)) + .unwrap_or_else(|| panic!("not run: {argvs:?}")) + }; + let untagged = at(&|argv| argv == UNTAG); + let up = at(&|argv| argv.get(1).map(String::as_str) == Some("up")); + assert!(untagged < up, "the tag was removed after the up"); + assert!( + said.contains(&LaunchNotice::DerivedImageRemoved { + workspace_id: "myws".to_owned(), + image: "proj-8260d:devpod-9409".to_owned(), + }), + "{said:?}" + ); + } + + /// A tag that stays would make `dl --refresh-stale` recreate the workspace + /// on every run and never make it current, so it does not recreate it. + #[test] + fn an_unattended_recreate_that_cannot_remove_devpods_tag_does_not_run() { + let scene = Scene::new() + .with_running("myws") + .with_stale_derived_image("myws"); + scene.runner.script( + UNTAG, + Response::failed(1, "Error response from daemon: busy\n"), + ); + + let (launched, _) = recreate(&scene, &LaunchVerb::RecreateUnattended); + + assert_eq!( + launched, + Ok(Launched::Refused(LaunchRefusal::DerivedImageKept( + DerivedImageKept { + workspace_id: "myws".to_owned(), + image: "proj-8260d:devpod-9409".to_owned(), + why: "Error response from daemon: busy".to_owned(), + } + ))) + ); + assert!( + !scene + .runner + .argvs() + .iter() + .any(|argv| argv.get(1).map(String::as_str) == Some("up")), + "the recreate ran" + ); + } + + /// A recreate the user asked for goes ahead, and says the workspace stays + /// on the older image. + #[test] + fn a_recreate_that_cannot_remove_devpods_tag_goes_ahead_and_says_so() { + let scene = Scene::new() + .with_running("myws") + .with_stale_derived_image("myws"); + scene.runner.script( + UNTAG, + Response::failed(1, "Error response from daemon: busy\n"), + ); + + let (launched, said) = recreate(&scene, &LaunchVerb::Recreate); + + assert_eq!( + launched, + Ok(Launched::Session(Session::RemoteExit { status: 0 })) + ); + assert!( + scene + .runner + .argvs() + .iter() + .any(|argv| argv.get(1).map(String::as_str) == Some("up")), + "the recreate did not run" + ); + assert!( + said.contains(&LaunchNotice::DerivedImageKept(DerivedImageKept { + workspace_id: "myws".to_owned(), + image: "proj-8260d:devpod-9409".to_owned(), + why: "Error response from daemon: busy".to_owned(), + })), + "{said:?}" + ); + } + + /// Outside herdr, `dl --refresh-stale` recreates only a workspace it has seen + /// no Claude process in, so the warning that the recreate ends unseen agents + /// would be false there. + #[test] + fn an_unattended_recreate_outside_herdr_does_not_warn_of_agents_it_ends() { + let scene = Scene::new().with_running("myws"); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let launched = { + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + launch.run("myws", &LaunchVerb::RecreateUnattended, None) + }; + + assert_eq!(launched, Ok(Launched::Ready)); + assert!( + !parts + .said + .iter() + .any(|notice| matches!(notice, LaunchNotice::AgentSessionsUnseen { .. })), + "{:?}", + parts.said + ); + } + /// A recreate whose `devpod up` the OS would not start never touched the /// container, so the agent in `w1:p1` still runs: nothing is typed and no line /// is handed over to start a second one. @@ -14075,6 +14682,24 @@ mod tests { assert!(up.contains(&"--reset".to_owned()), "{up:?}"); } + #[test] + fn only_the_two_recreates_recreate_and_only_refresh_stale_runs_unattended() { + // (verb, is_recreate, is_unattended) + for (verb, recreate, unattended) in [ + (LaunchVerb::Attach { command: None }, false, false), + (LaunchVerb::Up, false, false), + (LaunchVerb::Code, false, false), + (LaunchVerb::Recreate, true, false), + (LaunchVerb::RecreateUnattended, true, true), + (LaunchVerb::Reset, false, false), + (LaunchVerb::Restart, false, false), + (LaunchVerb::Dotfiles, false, false), + ] { + assert_eq!(verb.is_recreate(), recreate, "{verb:?}"); + assert_eq!(verb.is_unattended(), unattended, "{verb:?}"); + } + } + #[test] fn a_refused_up_warms_the_cache_for_up_and_code_and_for_nothing_else() { // dl.py 4779/4788: those two ask for the refresh *before* they read the diff --git a/rust/devlaunch-core/src/flows/lifecycle/tests.rs b/rust/devlaunch-core/src/flows/lifecycle/tests.rs index 5311436e..27d99631 100644 --- a/rust/devlaunch-core/src/flows/lifecycle/tests.rs +++ b/rust/devlaunch-core/src/flows/lifecycle/tests.rs @@ -91,7 +91,7 @@ fn devpod_home_recording( std::fs::write( &result, serde_json::json!({ - "ContainerDetails": { "Id": "container-id" }, + "ContainerDetails": { "ID": "container-id" }, "MergedConfig": {}, "SubstitutionContext": { "LocalWorkspaceFolder": local_workspace_folder, diff --git a/rust/devlaunch-core/src/flows/mod.rs b/rust/devlaunch-core/src/flows/mod.rs index ed6d7201..b5e2739a 100644 --- a/rust/devlaunch-core/src/flows/mod.rs +++ b/rust/devlaunch-core/src/flows/mod.rs @@ -34,6 +34,8 @@ pub mod pull_request; // binary surface — not part of the frozen wf API (#251 §7) pub mod records; // binary surface — not part of the frozen wf API (#251 §7) +pub mod refresh_stale; +// binary surface — not part of the frozen wf API (#251 §7) pub mod repo_manager; // binary surface — not part of the frozen wf API (#251 §7), and only just: the // reporting half is crate-private like `clients::herdr` whose decisions it diff --git a/rust/devlaunch-core/src/flows/refresh_stale.rs b/rust/devlaunch-core/src/flows/refresh_stale.rs new file mode 100644 index 00000000..00029e50 --- /dev/null +++ b/rust/devlaunch-core/src/flows/refresh_stale.rs @@ -0,0 +1,384 @@ +//! Which stale workspaces `dl --refresh-stale` may recreate now, and why it +//! leaves each of the others alone (devlaunch#673, part 3). +//! +//! [`crate::flows::stale_images`] says which containers run an older image than +//! their reference names. A recreate fixes that and ends every process in the +//! container. [`crate::flows::agent_sessions`] brings the agents back, so what is +//! left to lose is work in flight: an agent in the middle of a turn, a command +//! an agent started, a build. This module decides, per stale workspace, whether +//! there is any. [`plan`] decides and does nothing; [`run`] judges each +//! workspace again before its recreate, and the binary runs the recreates. +//! +//! # The rule +//! +//! A stale workspace is refreshed only when every one of these holds, checked in +//! this order, and the first that fails is the reason it is skipped: +//! +//! 1. **No `dl` is launching it.** Its launch lock +//! ([`crate::flows::launch_locks`]) is free. +//! 2. **Its processes can be read.** One `docker exec` as root reads +//! `/proc//stat` and `/proc//cmdline` for every process. A +//! container that does not answer (stopped, gone, no `sh`) is skipped: no +//! reading is no grounds to recreate. +//! 3. **No build runs in it.** A process whose name (`comm`) is one of +//! [`BUILD_PROGRAMS`] is a build. The list is short on purpose: compilers, +//! linkers and the build drivers seen on the hosts this was made for. +//! 4. **No agent runs a command in it.** An agent is a process of any agent dl +//! knows by name ([`herdr::agent_named`]): Claude, codex, gemini. One whose +//! child is a shell is running a tool call or a background task: the agents +//! run both through a shell. Claude's MCP servers are started directly, not +//! through a shell, so they do not count. +//! 5. **Every agent session in it can be brought back, and is idle.** With no +//! agent process there is nothing to bring back and this holds. Otherwise dl +//! has to see the sessions, which takes a herdr pane: outside herdr it skips. +//! herdr has to answer, every pane in the workspace with a live agent has to +//! have a saved line ([`agent_sessions::HeldSessions::unresumable`]), each +//! held agent has to be `idle` or `done`, and there must be no more agent +//! sessions in the container than herdr's panes hold. Both sides of that count +//! are agents of any kind. A session started from +//! a terminal outside herdr would otherwise end with nothing to start it again. +//! +//! The check is a moment's reading, and a recreate starts a moment later. A +//! session that starts working in between is ended like any recreate ends it. + +use std::path::Path; + +use crate::clients::docker; +use crate::clients::herdr; +use crate::domain::locks; +use crate::domain::proc_stat; +use crate::flows::agent_sessions; +use crate::flows::launch_locks::LaunchLocks; +use crate::flows::stale_images::StaleImages; +use crate::runner::Runner; + +/// The process names that mean a build runs, compared with the kernel's `comm` +/// (the first 15 bytes of the program's file name). +pub const BUILD_PROGRAMS: &[&str] = &[ + "bazel", "bazelisk", "cargo", "rustc", "cc1", "cc1plus", "make", "gmake", "ninja", "cmake", + "colcon", "gcc", "g++", "clang", "clang++", "ld", "ld.lld", "ld.gold", "collect2", +]; + +/// The shells an agent runs its tool calls and background tasks through. +const SHELLS: &[&str] = &["sh", "bash", "dash", "zsh", "fish"]; + +/// What `dl --refresh-stale` will do with one stale workspace. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Planned { + pub workspace_id: String, + /// The reference that now names a newer image. + pub reference: String, + pub verdict: Verdict, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Verdict { + /// Recreate it, and start its held agents again. + Refresh, + /// Leave it alone, for this reason. + Skip(Skip), +} + +/// Why a stale workspace is left alone. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Skip { + /// Another `dl` holds its launch lock. + LaunchUnderWay, + /// Its launch lock could not be asked. + LockUnreadable { why: String }, + /// Its container is stopped. A recreate would also start it, which a refresh + /// is not asked to do. + Stopped, + /// Its container's processes could not be read. + ProcessesUnread { why: String }, + /// A build runs in it. + Building { program: String, pid: u32 }, + /// An agent runs a command in it. + AgentRunsACommand { program: String, pid: u32 }, + /// An agent runs in it and this `dl` is not in a herdr pane, so it cannot see + /// the sessions to bring them back. + SessionsUnseen, + /// herdr did not say which panes it has. + HerdrUnanswered, + /// This pane holds a live agent in the workspace and no line to start it again. + Unresumable { pane_id: String }, + /// The agent in this pane is not idle. + AgentBusy { pane_id: String, state: AgentState }, + /// More agent sessions run in the container than herdr's panes hold. + SessionsOutsidePanes { running: usize, held: usize }, +} + +/// What a held agent is doing, when it is not something a refresh may end. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum AgentState { + Working, + /// Waiting on a permission prompt. + Blocked, + /// herdr said a state this build does not know. + Unknown, + /// herdr did not answer for the pane. + Unanswered, +} + +/// What to do with each stale workspace among `workspace_ids`, in their order. +/// +/// A workspace `stale` does not name is not stale and is left out. `manager` is +/// the herdr this `dl` runs in, `None` outside one; `read` reads herdr's saved +/// session file, as [`agent_sessions::collect`] takes it. +pub fn plan<'w>( + runner: &dyn Runner, + stale: &StaleImages, + workspace_ids: impl IntoIterator, + manager: Option<&agent_sessions::HerdrPanes>, + launch_locks: &LaunchLocks, + read: &dyn Fn(&Path) -> Option, +) -> Vec { + workspace_ids + .into_iter() + .filter_map(|workspace_id| { + let image = stale.of(workspace_id)?; + let verdict = match judge( + runner, + workspace_id, + image.container(), + manager, + launch_locks, + read, + ) { + Some(skip) => Verdict::Skip(skip), + None => Verdict::Refresh, + }; + Some(Planned { + workspace_id: workspace_id.to_owned(), + reference: image.reference().to_owned(), + verdict, + }) + }) + .collect() +} + +/// What `dl --refresh-stale` did with its stale workspaces: each one is counted +/// once, as refreshed, skipped, or failed. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct Tally { + pub refreshed: usize, + pub skipped: usize, + pub failed: usize, +} + +/// The side of a refresh that [`run`] leaves to its caller: saying the plan, +/// saying a second-look skip, and running each recreate. +pub trait Refresher { + /// The whole plan, before the first recreate. + fn planned(&mut self, plan: &[Planned]); + /// A workspace the plan would refresh, left alone on its second look. + fn left_alone(&mut self, workspace_id: &str, skip: &Skip); + /// Recreate the workspace; `true` when the recreate succeeded. + fn recreate(&mut self, workspace_id: &str) -> bool; +} + +/// Plan the refresh of the stale workspaces among `workspace_ids` (as [`plan`] +/// does, with the same inputs), then recreate each one the plan would refresh. +/// +/// The recreates before one can take minutes, so each workspace is judged +/// again just before its recreate, and a skip found then wins: it is left +/// alone and said through [`Refresher::left_alone`]. A failed recreate is +/// counted and the run goes on to the next. +pub fn run<'w>( + runner: &dyn Runner, + stale: &StaleImages, + workspace_ids: impl IntoIterator, + manager: Option<&agent_sessions::HerdrPanes>, + launch_locks: &LaunchLocks, + read: &dyn Fn(&Path) -> Option, + refresher: &mut dyn Refresher, +) -> Tally { + let planned = plan(runner, stale, workspace_ids, manager, launch_locks, read); + refresher.planned(&planned); + let mut tally = Tally::default(); + for planned in &planned { + if let Verdict::Skip(_) = planned.verdict { + tally.skipped += 1; + continue; + } + let id = planned.workspace_id.as_str(); + // The plan named it, so `stale` does. + let Some(image) = stale.of(id) else { + tally.skipped += 1; + continue; + }; + if let Some(skip) = judge(runner, id, image.container(), manager, launch_locks, read) { + refresher.left_alone(id, &skip); + tally.skipped += 1; + continue; + } + if refresher.recreate(id) { + tally.refreshed += 1; + } else { + tally.failed += 1; + } + } + tally +} + +/// The first rule this workspace fails, or `None` when it may be refreshed. +fn judge( + runner: &dyn Runner, + workspace_id: &str, + container: &str, + manager: Option<&agent_sessions::HerdrPanes>, + launch_locks: &LaunchLocks, + read: &dyn Fn(&Path) -> Option, +) -> Option { + match locks::run_if_lock_free(&launch_locks.path_for(workspace_id), || ()) { + Ok(Some(())) => {} + Ok(None) => return Some(Skip::LaunchUnderWay), + Err(error) => { + return Some(Skip::LockUnreadable { + why: crate::flows::launch::lock_reason(&error), + }); + } + } + let table = match docker::exec_as_root(runner, container, &["sh", "-c", READ_THE_TABLE]) { + Ok(out) => match process_table(&out) { + Ok(table) => table, + Err(why) => return Some(Skip::ProcessesUnread { why }), + }, + Err(docker::ExecFailure::NotRunning) => return Some(Skip::Stopped), + Err(docker::ExecFailure::Other(why)) => return Some(Skip::ProcessesUnread { why }), + }; + if let Some(build) = table + .iter() + .find(|process| BUILD_PROGRAMS.contains(&process.comm.as_str())) + { + return Some(Skip::Building { + program: build.comm.clone(), + pid: build.pid, + }); + } + let is_agent = |pid: u32| { + table + .iter() + .any(|process| process.pid == pid && process.is_agent()) + }; + if let Some(command) = table + .iter() + .find(|process| SHELLS.contains(&process.comm.as_str()) && is_agent(process.ppid)) + { + return Some(Skip::AgentRunsACommand { + program: command.comm.clone(), + pid: command.pid, + }); + } + let running = table + .iter() + .filter(|process| process.is_agent() && !is_agent(process.ppid)) + .count(); + if running == 0 { + return None; + } + sessions_skip(runner, workspace_id, running, manager, read) +} + +/// Rule 5: the agent sessions in the workspace, as herdr's panes hold them. +fn sessions_skip( + runner: &dyn Runner, + workspace_id: &str, + running: usize, + manager: Option<&agent_sessions::HerdrPanes>, + read: &dyn Fn(&Path) -> Option, +) -> Option { + let Some(manager) = manager else { + return Some(Skip::SessionsUnseen); + }; + let Some(held) = agent_sessions::collect(runner, manager, workspace_id, read) else { + return Some(Skip::HerdrUnanswered); + }; + if let Some(pane) = held.unresumable.first() { + return Some(Skip::Unresumable { + pane_id: pane.pane_id.clone(), + }); + } + for session in &held.sessions { + let state = match agent_sessions::agent_reading(runner, manager, &session.pane_id) { + herdr::AgentReading::Agent(agent) => match agent.status { + herdr::AgentStatus::Idle | herdr::AgentStatus::Done => continue, + herdr::AgentStatus::Working => AgentState::Working, + herdr::AgentStatus::Blocked => AgentState::Blocked, + herdr::AgentStatus::Unknown => AgentState::Unknown, + }, + herdr::AgentReading::NoAgent => continue, + herdr::AgentReading::Unanswered => AgentState::Unanswered, + }; + return Some(Skip::AgentBusy { + pane_id: session.pane_id.clone(), + state, + }); + } + (running > held.sessions.len()).then_some(Skip::SessionsOutsidePanes { + running, + held: held.sessions.len(), + }) +} + +/// What runs inside the container to print its processes: for each, its `stat` +/// line, then its `cmdline` with every NUL and newline made a unit separator +/// (0x1f), one process per two lines. A closing marker says the script ran to +/// its end. `sh`, `cat` and `tr` are in every image `dl` opens. +const READ_THE_TABLE: &str = "for d in /proc/[0-9]*; do \ + s=$(cat \"$d/stat\" 2>/dev/null) || continue; \ + c=$(tr '\\000\\n' '\\037\\037' < \"$d/cmdline\" 2>/dev/null); \ + printf '%s\\n%s\\n' \"$s\" \"$c\"; done; echo devlaunch-end"; + +/// One process, as much of it as the rule reads. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Process { + pid: u32, + ppid: u32, + comm: String, + argv: Vec, +} + +impl Process { + /// An agent dl knows by name ([`herdr::agent_named`]): named for one, + /// started as one, or a `node` running a script named for one (the npm + /// install). + fn is_agent(&self) -> bool { + let named = |word: &String| herdr::agent_named(word).is_some(); + herdr::agent_named(&self.comm).is_some() || self.argv.iter().take(2).any(named) + } +} + +/// Read what [`READ_THE_TABLE`] printed. A zombie is not running anything, so +/// it is left out. +fn process_table(out: &str) -> Result, String> { + let body = out + .strip_suffix("devlaunch-end\n") + .ok_or("the listing stopped before its end")?; + let lines: Vec<&str> = body.lines().collect(); + let mut table = Vec::new(); + for pair in lines.chunks(2) { + let [stat, cmdline] = pair else { + return Err("the listing ended halfway through a process".to_owned()); + }; + let stat = + proc_stat::parse(stat).ok_or_else(|| format!("a stat line did not parse: {stat}"))?; + if stat.zombie { + continue; + } + table.push(Process { + pid: stat.pid, + ppid: stat.ppid, + comm: stat.comm, + argv: cmdline + .split('\u{1f}') + .filter(|word| !word.is_empty()) + .map(str::to_owned) + .collect(), + }); + } + Ok(table) +} + +#[cfg(test)] +mod tests; diff --git a/rust/devlaunch-core/src/flows/refresh_stale/tests.rs b/rust/devlaunch-core/src/flows/refresh_stale/tests.rs new file mode 100644 index 00000000..c050d2b6 --- /dev/null +++ b/rust/devlaunch-core/src/flows/refresh_stale/tests.rs @@ -0,0 +1,781 @@ +use super::*; +use crate::testing::ScriptedRunner; +use devlaunch_test_support::Response; + +const HERDR: &str = "/opt/herdr/bin/herdr"; + +fn stale() -> StaleImages { + // `of_pairs` names each container after its workspace. + StaleImages::of_pairs([("myws", "ghcr.io/o/img:latest")]) +} + +/// One process as the in-container script prints it: its stat line and its +/// argv joined by unit separators. +fn process(pid: u32, ppid: u32, comm: &str, argv: &[&str]) -> String { + format!( + "{pid} ({comm}) S {ppid} {pid} {pid} 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 1000 1000 10 \ + 18446744073709551615 0 0 0 0 0 0 0 0 0 0 0 0 17 3 0 0 0 0 0\n{}\n", + argv.join("\u{1f}") + ) +} + +fn table(processes: &[String]) -> String { + format!("{}devlaunch-end\n", processes.concat()) +} + +fn quiet() -> Vec { + vec![ + process(1, 0, "sleep", &["sleep", "infinity"]), + process(40, 1, "sshd", &["sshd: vscode"]), + process(41, 40, "bash", &["-bash"]), + ] +} + +fn with_claude() -> Vec { + let mut processes = quiet(); + processes.push(process( + 50, + 41, + "claude", + &["claude", "--remote-control=myws", "--resume", "4b1e"], + )); + // An MCP server: started directly, not through a shell. + processes.push(process(51, 50, "node", &["node", "/usr/lib/mcp/server.js"])); + processes +} + +fn runner_reading(processes: &[String]) -> ScriptedRunner { + ScriptedRunner::new().with_script( + ["docker", "exec", "--user", "0", "myws", "sh", "-c"], + Response::stdout(table(processes)), + ) +} + +fn plan_of( + runner: &ScriptedRunner, + manager: Option<&agent_sessions::HerdrPanes>, + cache: &Path, + session: Option, +) -> Vec { + plan( + runner, + &stale(), + ["myws", "current"], + manager, + &LaunchLocks::under(cache), + &|_| session.clone(), + ) + .into_iter() + .map(|planned| { + assert_eq!(planned.workspace_id, "myws"); + assert_eq!(planned.reference, "ghcr.io/o/img:latest"); + planned.verdict + }) + .collect() +} + +fn cache() -> tempfile::TempDir { + tempfile::tempdir().expect("a cache dir") +} + +#[test] +fn a_stale_workspace_where_nothing_runs_is_refreshed_and_a_current_one_is_left_out() { + let runner = runner_reading(&quiet()); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Refresh] + ); +} + +#[test] +fn each_build_program_is_a_reason_to_skip() { + for program in BUILD_PROGRAMS { + let mut processes = quiet(); + processes.push(process(77, 41, program, &[program, "build"])); + let runner = runner_reading(&processes); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::Building { + program: (*program).to_owned(), + pid: 77, + })], + "{program}" + ); + } +} + +/// A finished compiler its parent has not reaped yet is not a build. +#[test] +fn a_zombie_is_not_a_build() { + let mut processes = quiet(); + processes.push( + "77 (cc1plus) Z 41 77 77 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 1000 0 0\n\n".to_owned(), + ); + let runner = runner_reading(&processes); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Refresh] + ); +} + +#[test] +fn a_shell_under_claude_is_a_command_it_runs() { + let mut processes = with_claude(); + processes.push(process(60, 50, "bash", &["/bin/bash", "-c", "sleep 600"])); + let runner = runner_reading(&processes); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::AgentRunsACommand { + program: "bash".to_owned(), + pid: 60, + })] + ); +} + +/// The npm install runs Claude as `node .../claude`; its tool calls count too. +#[test] +fn a_claude_run_by_node_is_claude() { + let mut processes = quiet(); + processes.push(process( + 50, + 41, + "node", + &["node", "/usr/local/bin/claude", "--resume", "4b1e"], + )); + processes.push(process(60, 50, "sh", &["/bin/sh", "-c", "make"])); + let runner = runner_reading(&processes); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::AgentRunsACommand { + program: "sh".to_owned(), + pid: 60, + })] + ); +} + +/// codex runs its commands through a shell as Claude does. +#[test] +fn a_shell_under_codex_is_a_command_it_runs() { + let mut processes = quiet(); + processes.push(process(50, 41, "codex", &["codex", "resume", "019a"])); + processes.push(process(60, 50, "bash", &["/bin/bash", "-lc", "cargo test"])); + let runner = runner_reading(&processes); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::AgentRunsACommand { + program: "bash".to_owned(), + pid: 60, + })] + ); +} + +/// Each agent dl knows by name is a session to bring back, the npm form too. +#[test] +fn outside_herdr_a_workspace_running_any_agent_is_skipped() { + for (comm, argv) in [ + ("codex", ["codex", "resume"]), + ("node", ["node", "/usr/local/bin/codex"]), + ("node", ["node", "/usr/local/bin/gemini"]), + ] { + let mut processes = quiet(); + processes.push(process(50, 41, comm, &argv)); + let runner = runner_reading(&processes); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::SessionsUnseen)], + "{argv:?}" + ); + } +} + +#[test] +fn outside_herdr_a_workspace_running_claude_is_skipped() { + let runner = runner_reading(&with_claude()); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::SessionsUnseen)] + ); +} + +#[test] +fn a_stopped_container_is_skipped_as_stopped() { + // A recreate would also start it, which nobody asked a refresh to do. The + // skip names the reason, so the line does not read like a docker fault. + let runner = ScriptedRunner::new().with_script( + ["docker", "exec"], + Response::failed( + 1, + "Error response from daemon: container c1 is not running\n", + ), + ); + let cache = cache(); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::Stopped)] + ); +} + +#[test] +fn a_container_whose_processes_cannot_be_read_is_skipped() { + let runner = ScriptedRunner::new().with_script( + ["docker", "exec"], + Response::failed(1, "Error response from daemon: permission denied\n"), + ); + let cache = cache(); + + let verdicts = plan_of(&runner, None, cache.path(), None); + + let [Verdict::Skip(Skip::ProcessesUnread { why })] = verdicts.as_slice() else { + panic!("{verdicts:?}"); + }; + assert!(why.contains("permission denied"), "{why}"); +} + +#[test] +fn a_listing_cut_short_is_not_read_as_a_quiet_container() { + let runner = ScriptedRunner::new().with_script( + ["docker", "exec"], + Response::stdout(process(77, 41, "cc1plus", &["cc1plus"])), + ); + let cache = cache(); + + assert!(matches!( + plan_of(&runner, None, cache.path(), None).as_slice(), + [Verdict::Skip(Skip::ProcessesUnread { .. })] + )); +} + +#[test] +fn a_workspace_another_dl_is_launching_is_skipped_before_docker_is_asked() { + let runner = runner_reading(&quiet()); + let cache = cache(); + let locks = LaunchLocks::under(cache.path()); + let _held = locks::hold_lock(&locks.path_for("myws")).expect("the lock"); + + assert_eq!( + plan_of(&runner, None, cache.path(), None), + vec![Verdict::Skip(Skip::LaunchUnderWay)] + ); + assert!(runner.args_to("docker").is_empty()); +} + +// --- the sessions herdr holds + +fn manager() -> agent_sessions::HerdrPanes { + agent_sessions::HerdrPanes::resolve( + &herdr::HostEnv { + enabled: None, + in_pane: Some("1".to_owned()), + pane_id: Some("w1:p9".to_owned()), + socket: Some("/run/herdr/herdr.sock".to_owned()), + binary: Some(HERDR.to_owned()), + }, + Some(HERDR), + ) + .expect("a herdr pane") +} + +const CLAUDE_LINE: &str = + r#"["dl","myws","--","claude","--remote-control=myws","--resume","4b1e"]"#; + +/// herdr with pane `w1:p1` attached to `myws`, its saved line `line`, and +/// `status` as its agent's state. +fn herdr_holding(runner: &ScriptedRunner, line: Option<&str>, status: &str) -> String { + herdr_holding_agent(runner, "claude", line, status) +} + +/// [`herdr_holding`], with `agent` as herdr's name for the pane's agent. +fn herdr_holding_agent( + runner: &ScriptedRunner, + agent: &str, + line: Option<&str>, + status: &str, +) -> String { + runner.script( + [HERDR, "pane", "list"], + Response::stdout( + r#"{"id":"l","result":{"type":"pane_list","panes":[{"pane_id":"w1:p1","tab_id":"w1:t1","focused":false}]}}"#, + ), + ); + runner.script( + [HERDR, "pane", "process-info"], + Response::stdout( + r#"{"id":"p","result":{"type":"pane_process_info","process_info":{"foreground_processes":[{"argv":["devpod","ssh","myws"]}]}}}"#, + ), + ); + runner.script( + [HERDR, "agent", "get", "w1:p1"], + Response::stdout(format!( + r#"{{"id":"a","result":{{"type":"agent","agent":{{"agent":"{agent}","agent_status":"{status}"}}}}}}"# + )), + ); + let resume = line.map_or(String::new(), |argv| { + format!(r#","agent_resume":{{"argv":{argv}}}"#) + }); + format!( + r#"{{"workspaces":[{{"id":"w1","public_pane_numbers":{{"1":1}},"tabs":[{{"panes":{{"1":{{"cwd":"/"{resume}}}}}}}]}}]}}"# + ) +} + +#[test] +fn an_idle_claude_herdr_holds_a_line_for_is_refreshed() { + for status in ["idle", "done"] { + let runner = runner_reading(&with_claude()); + let session = herdr_holding(&runner, Some(CLAUDE_LINE), status); + let cache = cache(); + + let verdicts = plan_of(&runner, Some(&manager()), cache.path(), Some(session)); + + // herdr's `done` is `idle` that nobody has looked at yet, so both are an + // agent waiting for the user, and the recreate brings it back. + assert_eq!(verdicts, vec![Verdict::Refresh], "{status}"); + } +} + +#[test] +fn an_agent_that_is_not_idle_is_skipped_with_what_it_is_doing() { + for (status, state) in [ + ("working", AgentState::Working), + ("blocked", AgentState::Blocked), + ("unknown", AgentState::Unknown), + ] { + let runner = runner_reading(&with_claude()); + let session = herdr_holding(&runner, Some(CLAUDE_LINE), status); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Skip(Skip::AgentBusy { + pane_id: "w1:p1".to_owned(), + state, + })], + "{status}" + ); + } +} + +#[test] +fn a_live_agent_with_no_saved_line_is_skipped() { + let runner = runner_reading(&with_claude()); + let session = herdr_holding(&runner, None, "idle"); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Skip(Skip::Unresumable { + pane_id: "w1:p1".to_owned(), + })] + ); +} + +/// A second Claude started from a terminal outside herdr would end with +/// nothing to start it again. +#[test] +fn more_claude_sessions_than_herdr_holds_is_a_skip() { + let mut processes = with_claude(); + processes.push(process(52, 41, "claude", &["claude"])); + let runner = runner_reading(&processes); + let session = herdr_holding(&runner, Some(CLAUDE_LINE), "idle"); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Skip(Skip::SessionsOutsidePanes { + running: 2, + held: 1, + })] + ); +} + +/// herdr's panes hold agents of any kind, so the processes they are counted +/// against are too: a held codex does not cover a Claude outside herdr. +#[test] +fn a_held_codex_does_not_cover_a_claude_outside_herdr() { + let mut processes = with_claude(); + processes.push(process(70, 41, "codex", &["codex", "resume", "019a"])); + let runner = runner_reading(&processes); + let session = herdr_holding_agent( + &runner, + "codex", + Some(r#"["dl","myws","--","codex","resume","019a"]"#), + "idle", + ); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Skip(Skip::SessionsOutsidePanes { + running: 2, + held: 1, + })] + ); +} + +#[test] +fn a_herdr_that_does_not_list_its_panes_is_a_skip() { + let runner = runner_reading(&with_claude()); + runner.script([HERDR, "pane", "list"], Response::exited(1)); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), None), + vec![Verdict::Skip(Skip::HerdrUnanswered)] + ); +} + +/// A stat line's name sits in parentheses and may hold spaces and parentheses +/// of its own, so the parse takes the last `) ` as its end. A process whose +/// cmdline is empty (a kernel thread, or one that exited mid-read) has no argv. +#[test] +fn the_table_reads_what_the_script_prints() { + let printed = "1 (sleep) S 0 1 1 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 1 1\n\ + sleep\u{1f}infinity\u{1f}\n\ + 12 (a (b) c) R 1 12 1 34816 12 4194304 101 0 0 0 0 0 0 0 20 0 1 0 2 1\n\ + \n\ + devlaunch-end\n"; + + assert_eq!( + process_table(printed), + Ok(vec![ + Process { + pid: 1, + ppid: 0, + comm: "sleep".to_owned(), + argv: vec!["sleep".to_owned(), "infinity".to_owned()], + }, + Process { + pid: 12, + ppid: 1, + comm: "a (b) c".to_owned(), + argv: Vec::new(), + }, + ]) + ); +} + +#[test] +fn a_listing_that_ends_halfway_through_a_process_is_unread() { + let runner = ScriptedRunner::new().with_script( + ["docker", "exec"], + Response::stdout("1 (sleep) S 0 1 1 0 -1 4194560\ndevlaunch-end\n"), + ); + let cache = cache(); + + assert!(matches!( + plan_of(&runner, None, cache.path(), None).as_slice(), + [Verdict::Skip(Skip::ProcessesUnread { .. })] + )); +} + +#[test] +fn a_stat_line_that_does_not_parse_is_unread() { + let runner = ScriptedRunner::new().with_script( + ["docker", "exec"], + Response::stdout("x (y\n\ndevlaunch-end\n"), + ); + let cache = cache(); + + let verdicts = plan_of(&runner, None, cache.path(), None); + + let [Verdict::Skip(Skip::ProcessesUnread { why })] = verdicts.as_slice() else { + panic!("{verdicts:?}"); + }; + assert!(why.contains("did not parse"), "{why}"); +} + +/// `collect` keeps a saved line when `agent get` goes unanswered, so the +/// pane's agent is then one whose state nobody knows. +#[test] +fn a_held_agent_herdr_does_not_answer_for_is_skipped() { + let runner = runner_reading(&with_claude()); + runner.script([HERDR, "agent", "get"], Response::exited(1)); + let session = herdr_holding(&runner, Some(CLAUDE_LINE), "idle"); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Skip(Skip::AgentBusy { + pane_id: "w1:p1".to_owned(), + state: AgentState::Unanswered, + })] + ); +} + +/// herdr names no agent in the pane, so its saved line is not held, and the +/// Claude process in the container is one no pane holds. +#[test] +fn a_saved_line_in_a_pane_herdr_says_has_no_agent_holds_no_session() { + let runner = runner_reading(&with_claude()); + runner.script( + [HERDR, "agent", "get"], + Response::stdout(r#"{"id":"a","result":{"type":"agent","agent":null}}"#), + ); + let session = herdr_holding(&runner, Some(CLAUDE_LINE), "idle"); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Skip(Skip::SessionsOutsidePanes { + running: 1, + held: 0, + })] + ); +} + +#[test] +fn a_lock_that_cannot_be_asked_is_a_skip() { + let runner = runner_reading(&quiet()); + let cache = cache(); + std::fs::write( + cache + .path() + .join(crate::flows::launch_locks::LAUNCH_LOCK_DIR), + "", + ) + .expect("a file where the lock directory goes"); + + assert!(matches!( + plan_of(&runner, None, cache.path(), None).as_slice(), + [Verdict::Skip(Skip::LockUnreadable { .. })] + )); + assert!(runner.args_to("docker").is_empty()); +} + +/// A Claude started by a Claude (a subagent) is the same session. +#[test] +fn a_claude_under_a_claude_is_one_session() { + let mut processes = with_claude(); + processes.push(process(53, 50, "claude", &["claude", "-p", "review"])); + let runner = runner_reading(&processes); + let session = herdr_holding(&runner, Some(CLAUDE_LINE), "idle"); + let cache = cache(); + + assert_eq!( + plan_of(&runner, Some(&manager()), cache.path(), Some(session)), + vec![Verdict::Refresh] + ); +} + +#[test] +fn each_stale_workspace_gets_its_own_verdict() { + let mut building = quiet(); + building.push(process(77, 41, "cargo", &["cargo", "build"])); + let runner = ScriptedRunner::new() + .with_script( + ["docker", "exec", "--user", "0", "busy", "sh", "-c"], + Response::stdout(table(&building)), + ) + .with_script( + ["docker", "exec", "--user", "0", "quiet", "sh", "-c"], + Response::stdout(table(&quiet())), + ); + let cache = cache(); + let locks = LaunchLocks::under(cache.path()); + let _held = locks::hold_lock(&locks.path_for("locked")).expect("the lock"); + + let verdicts: Vec<(String, Verdict)> = plan( + &runner, + &StaleImages::of_pairs([ + ("locked", "ghcr.io/o/img:latest"), + ("busy", "ghcr.io/o/img:latest"), + ("quiet", "ghcr.io/o/img:latest"), + ]), + ["locked", "busy", "quiet"], + None, + &locks, + &|_| None, + ) + .into_iter() + .map(|planned| (planned.workspace_id, planned.verdict)) + .collect(); + + assert_eq!( + verdicts, + vec![ + ("locked".to_owned(), Verdict::Skip(Skip::LaunchUnderWay)), + ( + "busy".to_owned(), + Verdict::Skip(Skip::Building { + program: "cargo".to_owned(), + pid: 77, + }) + ), + ("quiet".to_owned(), Verdict::Refresh), + ] + ); +} + +#[test] +fn a_command_name_that_holds_a_closing_parenthesis_reads_whole() { + // A process may name itself anything up to 15 bytes, `) ` included, so the + // name ends at the last `)` on the line. + let out = table(&[process(1234, 1, "a) b", &["a"])]); + + let read = process_table(&out).expect("the line reads"); + + assert_eq!( + read.iter() + .map(|process| (process.pid, process.ppid, process.comm.as_str())) + .collect::>(), + [(1234, 1, "a) b")] + ); +} + +// --- the run: a second look before each recreate, and the tally + +/// A [`Refresher`] that records what it was told, fails the recreates it is +/// told to, and, when one named workspace is recreated, takes the launch lock +/// of another, as a second `dl` starting a launch meanwhile would. +struct Scripted<'l> { + locks: &'l LaunchLocks, + lock_on_recreate: Option<(&'static str, &'static str)>, + failing: &'static [&'static str], + held: Vec, + said: Vec, +} + +impl<'l> Scripted<'l> { + fn new(locks: &'l LaunchLocks) -> Self { + Self { + locks, + lock_on_recreate: None, + failing: &[], + held: Vec::new(), + said: Vec::new(), + } + } +} + +impl Refresher for Scripted<'_> { + fn planned(&mut self, plan: &[Planned]) { + for planned in plan { + self.said.push(format!( + "plan {} {:?}", + planned.workspace_id, planned.verdict + )); + } + } + + fn left_alone(&mut self, workspace_id: &str, skip: &Skip) { + self.said + .push(format!("left alone {workspace_id} {skip:?}")); + } + + fn recreate(&mut self, workspace_id: &str) -> bool { + self.said.push(format!("recreate {workspace_id}")); + if let Some((when, lock)) = self.lock_on_recreate + && when == workspace_id + { + self.held + .push(locks::hold_lock(&self.locks.path_for(lock)).expect("the lock")); + } + !self.failing.contains(&workspace_id) + } +} + +fn quiet_runner(ids: &[&str]) -> ScriptedRunner { + ids.iter().fold(ScriptedRunner::new(), |runner, id| { + runner.with_script( + ["docker", "exec", "--user", "0", id, "sh", "-c"], + Response::stdout(table(&quiet())), + ) + }) +} + +fn stale_of(ids: &[&'static str]) -> StaleImages { + StaleImages::of_pairs(ids.iter().map(|id| (*id, "ghcr.io/o/img:latest"))) +} + +#[test] +fn a_workspace_that_turns_busy_after_the_plan_is_left_alone_not_recreated() { + let ids = ["first", "second"]; + let runner = quiet_runner(&ids); + let cache = cache(); + let launch_locks = LaunchLocks::under(cache.path()); + let mut refresher = Scripted::new(&launch_locks); + refresher.lock_on_recreate = Some(("first", "second")); + + let tally = run( + &runner, + &stale_of(&ids), + ids, + None, + &launch_locks, + &|_| None, + &mut refresher, + ); + + assert_eq!( + refresher.said, + vec![ + "plan first Refresh", + "plan second Refresh", + "recreate first", + "left alone second LaunchUnderWay", + ] + ); + assert_eq!( + tally, + Tally { + refreshed: 1, + skipped: 1, + failed: 0, + } + ); +} + +#[test] +fn the_tally_counts_each_workspace_once_as_refreshed_skipped_or_failed() { + let ids = ["locked", "fine", "broken", "also_fine", "turns_busy"]; + let runner = quiet_runner(&ids); + let cache = cache(); + let launch_locks = LaunchLocks::under(cache.path()); + let _held = locks::hold_lock(&launch_locks.path_for("locked")).expect("the lock"); + let mut refresher = Scripted::new(&launch_locks); + refresher.failing = &["broken"]; + refresher.lock_on_recreate = Some(("also_fine", "turns_busy")); + + let tally = run( + &runner, + &stale_of(&ids), + ids.iter().copied().chain(["current"]), + None, + &launch_locks, + &|_| None, + &mut refresher, + ); + + assert_eq!( + tally, + Tally { + refreshed: 2, + skipped: 2, + failed: 1, + } + ); + assert_eq!( + refresher + .said + .iter() + .filter(|line| line.starts_with("recreate ")) + .collect::>(), + vec!["recreate fine", "recreate broken", "recreate also_fine"] + ); +} diff --git a/rust/devlaunch-core/src/flows/stale_images.rs b/rust/devlaunch-core/src/flows/stale_images.rs index 55bda8b1..c94eb12b 100644 --- a/rust/devlaunch-core/src/flows/stale_images.rs +++ b/rust/devlaunch-core/src/flows/stale_images.rs @@ -62,6 +62,13 @@ use crate::runner::Runner; #[derive(Clone, Debug, PartialEq, Eq)] pub struct StaleImage { reference: String, + /// The container devpod recorded for the workspace: the one that runs the + /// older image. + container: String, + /// The tag devpod built the container's image under, from the reference: + /// `:devpod-`. devpod names it by a hash of the config, not of + /// the reference, so a recreate reuses it and stays stale until it is gone. + derived: Option, } impl StaleImage { @@ -71,6 +78,16 @@ impl StaleImage { pub fn reference(&self) -> &str { &self.reference } + + /// The id of the container that runs the older image. + pub fn container(&self) -> &str { + &self.container + } + + /// The tag devpod built the older image under, when it built one. + pub fn derived_image(&self) -> Option<&str> { + self.derived.as_deref() + } } /// The stale workspaces among the ones asked about, by workspace id. @@ -104,6 +121,8 @@ impl StaleImages { id.to_owned(), StaleImage { reference: reference.to_owned(), + container: id.to_owned(), + derived: None, }, ) }) @@ -128,10 +147,10 @@ struct Created { #[derive(Clone, Debug, PartialEq, Eq)] struct Image { id: String, - /// docker's `Created`, cut to whole seconds: `YYYY-MM-DDTHH:MM:SS`. docker - /// writes it in UTC with a varying count of fractional digits, so the cut is - /// what makes two of them compare in time order as strings. - created: String, + /// docker's `Created`, parsed. docker does not always write it in UTC: with + /// the containerd image store a pulled image reads `...Z` and a local build + /// `...+01:00`, so a cut-and-compare of the text is off by the offset. + created: jiff::Timestamp, layers: Vec, } @@ -168,13 +187,26 @@ pub fn stale_images<'w>( let by_workspace = created .into_iter() .filter_map(|(id, created)| { - let running = running_images.get(running.get(&created.container)?)?; + let running_id = running.get(&created.container)?; let current = references.get(&created.reference)?; - is_stale(&created, running, current).then(|| { + let stale = match running_images.get(running_id) { + Some(running) => is_stale(&created, running, current), + // With the containerd image store, a local build whose only tag + // moved is gone from `docker image inspect` while a container + // still runs it. Its id is enough when the container was made + // from the reference itself; a derived image needs its layers. + None => created.created_from == created.reference && *running_id != current.id, + }; + stale.then(|| { + let derived = (created.created_from != created.reference + && is_devpod_tag(&created.created_from)) + .then_some(created.created_from); ( id.to_owned(), StaleImage { reference: created.reference, + container: created.container, + derived, }, ) }) @@ -183,6 +215,13 @@ pub fn stale_images<'w>( StaleImages { by_workspace } } +/// Whether `reference` is a tag devpod built: `:devpod-`. +fn is_devpod_tag(reference: &str) -> bool { + reference + .rsplit_once(':') + .is_some_and(|(name, tag)| !name.is_empty() && tag.starts_with("devpod-")) +} + /// Whether a container created as `created` and running `running` is behind /// the image its reference names now, `current`. fn is_stale(created: &Created, running: &Image, current: &Image) -> bool { @@ -203,7 +242,7 @@ fn created(devpod_home: Option<&DevpodHome>, workspace_id: &str) -> Option) -> BTree id.to_owned(), Image { id: id.to_owned(), - created: created.get(..19)?.to_owned(), + created: created.parse().ok()?, layers, }, )) @@ -299,7 +338,7 @@ mod tests { std::fs::write( home.result("default", workspace_id), serde_json::json!({ - "ContainerDetails": { "Id": container, "Config": { "Image": created_from } }, + "ContainerDetails": { "ID": container, "Config": { "Image": created_from } }, "MergedConfig": merged, }) .to_string(), @@ -365,6 +404,8 @@ mod tests { stale.of("ws").map(StaleImage::reference), Some("ghcr.io/o/img:latest") ); + // The container is what `dl --refresh-stale` reads the processes of. + assert_eq!(stale.of("ws").map(StaleImage::container), Some("c1")); } #[test] @@ -570,6 +611,75 @@ mod tests { assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); } + /// With the containerd image store, a local build whose only tag moved is + /// gone from `docker image inspect` while the container still runs it. + fn running_image_gone(fake: &FakeRunner, id: &str) { + fake.script( + [ + "docker", + "inspect", + "--type", + "image", + "--format", + IMAGE_FORMAT, + id, + ], + Response::failed(1, format!("Error: No such image: {id}\n")), + ); + } + + #[test] + fn a_container_made_from_the_reference_is_stale_after_its_image_left_the_store() { + let home = home_with("ws", "dl-test:latest", Some("dl-test:latest")); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:old"); + running_image_gone(&fake, "sha256:old"); + inspect_image(&fake, "dl-test:latest", line("sha256:new", &["l1", "l2"])); + + let stale = stale_images(&fake, Some(&home), ["ws"]); + + assert_eq!( + stale.of("ws").map(StaleImage::reference), + Some("dl-test:latest") + ); + } + + #[test] + fn a_derived_image_that_left_the_store_is_not_judged_without_its_layers() { + let home = home_with("ws", "devpod-abc", Some("ghcr.io/o/img:latest")); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:derived"); + running_image_gone(&fake, "sha256:derived"); + inspect_image(&fake, "ghcr.io/o/img:latest", line("sha256:base2", &["l1"])); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + /// 10:10 at +01:00 is 09:10 UTC, so a base pulled at 09:30 UTC is the newer + /// one. Compared as text, "10:10" sorts after "09:30" and hid it. + #[test] + fn created_times_compare_across_utc_offsets() { + let home = home_with("ws", "devpod-abc", Some("ghcr.io/o/img:latest")); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:derived"); + inspect_image( + &fake, + "sha256:derived", + line_at( + "sha256:derived", + "2026-10-08T10:10:00.5+01:00", + &["l1", "f1"], + ), + ); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line_at("sha256:base2", "2026-10-08T09:30:00Z", &["l2"]), + ); + + assert!(stale_images(&fake, Some(&home), ["ws"]).of("ws").is_some()); + } + #[test] fn a_missing_container_costs_the_others_nothing() { // docker prints a line for every container it found and exits 1 for the @@ -603,4 +713,57 @@ mod tests { assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); assert!(fake.argvs().is_empty()); } + + /// A stale derived image whose base moved, as devpod names it on a real host. + fn stale_derived(created_from: &str) -> StaleImages { + let home = home_with("ws", created_from, Some("ghcr.io/o/img:latest")); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:derived"); + inspect_image( + &fake, + "sha256:derived", + line("sha256:derived", &["l1", "l2", "f1"]), + ); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line_at("sha256:base2", "2026-10-05T09:00:00Z", &["l1", "l3"]), + ); + stale_images(&fake, Some(&home), ["ws"]) + } + + /// devpod tags the image it builds by a hash of the config, so a recreate + /// reuses it. The tag is named so that the recreate can remove it first. + #[test] + fn a_stale_derived_image_names_the_tag_devpod_built_it_under() { + let stale = stale_derived("proj-8260d:devpod-9409a1b2"); + + assert_eq!( + stale.of("ws").and_then(StaleImage::derived_image), + Some("proj-8260d:devpod-9409a1b2") + ); + } + + /// Only a tag devpod made is named: dl removes what it names. + #[test] + fn an_image_devpod_did_not_tag_is_not_named_as_derived() { + let stale = stale_derived("ghcr.io/o/other:latest"); + + assert!(stale.of("ws").is_some()); + assert_eq!(stale.of("ws").and_then(StaleImage::derived_image), None); + } + + #[test] + fn a_container_made_from_the_reference_names_no_derived_image() { + let home = home_with("ws", "ghcr.io/o/img:latest", Some("ghcr.io/o/img:latest")); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:old"); + inspect_image(&fake, "sha256:old", line("sha256:old", &["l1"])); + inspect_image(&fake, "ghcr.io/o/img:latest", line("sha256:new", &["l2"])); + + let stale = stale_images(&fake, Some(&home), ["ws"]); + + assert!(stale.of("ws").is_some()); + assert_eq!(stale.of("ws").and_then(StaleImage::derived_image), None); + } } diff --git a/rust/devlaunch-core/src/lib.rs b/rust/devlaunch-core/src/lib.rs index aefb3692..d381e9ae 100644 --- a/rust/devlaunch-core/src/lib.rs +++ b/rust/devlaunch-core/src/lib.rs @@ -66,7 +66,7 @@ //! **What it still does not reach, and it is not one type.** A type [`api`] //! never re-exports but a promised signature names is reachable from outside //! and classified as binary surface, so a break in it diffs -//! `public-api.rest.txt` alone. Counted rather than guessed at: **40 such types +//! `public-api.rest.txt` alone. Counted rather than guessed at: **41 such types //! own close to six hundred rows over there**, and `scripts/public-api-snapshots.sh //! --print-residual` lists them. //! @@ -82,7 +82,7 @@ //! //! So a `public-api.rest.txt` diff is routine for a row whose subject nothing //! promised names, and a contract change for a row whose subject is one of the -//! 40. `--print-residual` is how you tell which you are looking at. +//! 41. `--print-residual` is how you tell which you are looking at. //! //! `scripts/public-api-snapshots.sh` regenerates both; see "The public-API //! snapshots" in docs/development.md. diff --git a/rust/dl/src/cli.rs b/rust/dl/src/cli.rs index a667f07b..f2e1867a 100644 --- a/rust/dl/src/cli.rs +++ b/rust/dl/src/cli.rs @@ -417,6 +417,8 @@ pub(crate) enum Command { Purge { yes: bool, }, + /// `dl --refresh-stale` — recreate each stale workspace that is safe to. + RefreshStale, /// `dl --herdr-shell`, and the `dl-herdr-shell` name that means the same. HerdrShell, HerdrShellReady { @@ -631,6 +633,12 @@ pub(crate) struct Cli { /// Remove devlaunch's workspaces and caches. #[arg(long, group = "what")] purge: bool, + /// Recreate each workspace that runs an older image than its reference, when + /// its agents are idle and nothing builds in it, and start its agents + /// again. Skips the others and says why. Attaches nothing, so it can run after + /// a pull. + #[arg(long = "refresh-stale", group = "what")] + refresh_stale: bool, /// Print the version. #[arg(long, group = "what")] version: bool, @@ -723,6 +731,7 @@ const GRAMMAR: &str = "Examples: dl stop blooop-devlaunch-main-1a2b Stop it by workspace id dl blooop/devlaunch --rm Open it, and delete it when the shell exits dl --ls --json Every workspace, machine-readable + dl --refresh-stale Recreate the stale workspaces left idle Workspace commands (dl , or dl ): up Start it without attaching @@ -856,6 +865,7 @@ enum Chosen { Prune, Reconcile, Purge, + RefreshStale, Version, Repos, ClaudeProfiles, @@ -879,6 +889,7 @@ impl Cli { (self.prune, Chosen::Prune), (self.reconcile, Chosen::Reconcile), (self.purge, Chosen::Purge), + (self.refresh_stale, Chosen::RefreshStale), (self.version, Chosen::Version), (self.repos, Chosen::Repos), (self.claude_profiles, Chosen::ClaudeProfiles), @@ -1053,6 +1064,7 @@ fn global_command(cli: &Cli, chosen: Chosen) -> Result { }, Chosen::Reconcile => Command::Reconcile { yes: cli.yes }, Chosen::Purge => Command::Purge { yes: cli.yes }, + Chosen::RefreshStale => Command::RefreshStale, Chosen::Version => Command::Version, Chosen::Repos => Command::Repos, Chosen::ClaudeProfiles => Command::ClaudeProfiles, @@ -1079,6 +1091,7 @@ fn flag_of(chosen: Chosen) -> &'static str { Chosen::Prune => "--prune", Chosen::Reconcile => "--reconcile", Chosen::Purge => "--purge", + Chosen::RefreshStale => "--refresh-stale", Chosen::Version => "--version", Chosen::Repos => "--repos", Chosen::ClaudeProfiles => "--claude-profiles", @@ -1845,6 +1858,35 @@ mod tests { ); } + /// devlaunch#673. Unattended, so there is no question for `-y` to answer, and + /// it acts on every stale workspace, so there is no workspace to name. + #[test] + fn refresh_stale_is_a_global_command_with_no_modifiers() { + assert_eq!(parse(&["--refresh-stale"]), Ok(Command::RefreshStale)); + assert_eq!( + parse(&["--refresh-stale", "ws"]), + Err(GrammarError::TargetNotAllowed { + command: "--refresh-stale" + }) + ); + assert_eq!( + parse(&["--refresh-stale", "-y"]), + Err(GrammarError::ModifierNotAllowed { + modifier: "--yes", + command: "--refresh-stale" + }) + ); + assert_eq!( + parse(&["--refresh-stale", "--force"]), + Err(GrammarError::ModifierNotAllowed { + modifier: "--force", + command: "--refresh-stale" + }) + ); + // Its own flag, not `--refresh` with a word after it. + assert_eq!(parse(&["--refresh"]), Ok(Command::Refresh)); + } + #[test] fn a_global_command_takes_no_workspace() { assert_eq!( diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index 3b48e83a..4742a768 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -16,12 +16,13 @@ use devlaunch_core::domain::config; use devlaunch_core::domain::spec::DevcontainerPath; use devlaunch_core::domain::workspace_id::WorkspaceId; use devlaunch_core::domain::xdg; +use devlaunch_core::flows::agent_sessions; use devlaunch_core::flows::claude_profiles; use devlaunch_core::flows::completion::{self, FileState, InstallError, Installed, RcChange}; use devlaunch_core::flows::completion_cache::{self, Refreshed}; use devlaunch_core::flows::kept_copies::KeptCopies; use devlaunch_core::flows::kill; -use devlaunch_core::flows::launch::{ColdPath, LaunchNotice}; +use devlaunch_core::flows::launch::{ColdPath, Host, LaunchNotice, LaunchVerb}; use devlaunch_core::flows::launch_locks::LaunchLocks; use devlaunch_core::flows::lifecycle::{ self, ChildWork, DeleteStalled, Insisted, Insistence, LifecycleNotice, PruneError, @@ -30,6 +31,7 @@ use devlaunch_core::flows::lifecycle::{ use devlaunch_core::flows::listing::{self, CommandContext, DlView, Sizes}; use devlaunch_core::flows::pull_request; use devlaunch_core::flows::records::{Records, StartupError, open_records, open_storage}; +use devlaunch_core::flows::refresh_stale; use devlaunch_core::flows::repo_manager::CacheNotice; use devlaunch_core::flows::session_manager::{self, PaneDestination}; use devlaunch_core::flows::stale_images::stale_images; @@ -133,6 +135,7 @@ pub(crate) fn dispatch( ), Command::Reconcile { yes } => render_reconcile(runner, &mut context, refresh, yes), Command::Purge { yes } => render_purge(&mut context, cache, yes), + Command::RefreshStale => render_refresh_stale(runner, &mut context, cache, refresh), // Re-entered rather than special-cased: a pane that resolved to a // workspace is a `dl `, and the whole point is that it is // indistinguishable from one typed by hand -- same launch, same terminal @@ -1266,6 +1269,106 @@ impl Cleanup { } } +/// `dl --refresh-stale`: recreate each stale workspace that is safe to recreate, +/// and start its agents again (devlaunch#673). +/// +/// It runs unattended, after a pull, so it asks nothing and attaches nothing. +/// It says the whole plan first, one line per stale workspace, so the skips are +/// on the screen before the recreates start to scroll it away. Which workspaces +/// are safe, and the second look before each recreate, are core's +/// ([`refresh_stale::run`]); each recreate is the launch flow's own, with its +/// own notices. +/// +/// A recreate that fails is said by its launch and counted, and the loop goes on +/// to the next. The exit status is 1 when any recreate failed. A skip is not a +/// failure: leaving a busy workspace alone is what the command is for. +fn render_refresh_stale<'r>( + runner: &'r dyn Runner, + context: &mut CommandContext<'r>, + cache: &Path, + refresh: &mut Refresh<'_>, +) -> Ending { + let workspaces = match context.workspaces() { + Err(refused) => return refuse_listing(&refused), + Ok(workspaces) => workspaces, + }; + let ids: Vec<&str> = workspaces + .iter() + .map(|workspace| workspace.id.as_str()) + .collect(); + let stale = stale_images(runner, DevpodHome::locate().as_ref(), ids.iter().copied()); + if stale.is_empty() { + println!("{}", render::REFRESH_STALE_NOTHING); + return Ending::Done; + } + let host = Host::from_process(cache); + let manager = agent_sessions::HerdrPanes::from_host(&host); + let mut opening = render::Saying; + let mut recreates = Recreates { + context, + cache, + refresh, + cold: ColdPath::new(runner, &mut opening), + }; + let tally = refresh_stale::run( + runner, + &stale, + ids.iter().copied(), + manager.as_ref(), + &LaunchLocks::under(cache), + &|path| std::fs::read_to_string(path).ok(), + &mut recreates, + ); + println!("{}", render::refresh_stale_summary(tally)); + if tally.failed == 0 { + Ending::Done + } else { + Ending::Refused + } +} + +/// The binary's side of `dl --refresh-stale`: it says the plan and each +/// second-look skip, and runs each recreate as the launch flow's own. +struct Recreates<'c, 'r, 'f, 'o> { + context: &'c mut CommandContext<'r>, + cache: &'c Path, + refresh: &'c mut Refresh<'f>, + cold: ColdPath<'r, 'o>, +} + +impl refresh_stale::Refresher for Recreates<'_, '_, '_, '_> { + fn planned(&mut self, plan: &[refresh_stale::Planned]) { + for line in plan.iter().map(render::refresh_stale_line) { + println!("{line}"); + } + } + + fn left_alone(&mut self, workspace_id: &str, skip: &refresh_stale::Skip) { + println!( + "{}", + render::refresh_stale_rejudged_line(workspace_id, skip) + ); + } + + fn recreate(&mut self, workspace_id: &str) -> bool { + let ran = launch::render_launch( + self.context, + self.cache, + self.refresh, + &mut self.cold, + workspace_id, + &LaunchVerb::RecreateUnattended, + None, + None, + None, + None, + ); + // No `refresh.rearm()`: a recreate keeps the workspace's id, so the + // completion cache the first one refreshed holds for the rest. + ran.ending == Ending::Done + } +} + /// `dl --purge [-y]` — the workspaces devlaunch created, and its whole cache. /// /// Every ending it *returns* goes through the docker boundary, including the abort: diff --git a/rust/dl/src/launch.rs b/rust/dl/src/launch.rs index 38f04ac1..37663535 100644 --- a/rust/dl/src/launch.rs +++ b/rust/dl/src/launch.rs @@ -317,7 +317,8 @@ fn ran(outcome: Result, cache: &Path) -> Ran { | LaunchRefusal::UnknownWorkspace { .. } | LaunchRefusal::IdCollision { .. } | LaunchRefusal::BranchNotNamed { .. } - | LaunchRefusal::NotPrepared { .. } => Ran { + | LaunchRefusal::NotPrepared { .. } + | LaunchRefusal::DerivedImageKept(_) => Ran { ending: Ending::Refused, reached: Reached::Nothing, }, diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index 64405586..b7a76235 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -20,6 +20,7 @@ use devlaunch_core::clients::gh::{ }; use devlaunch_core::clients::git::Failure as GitFailure; use devlaunch_core::clients::ssh::{NotRun as SshNotRun, UnsafeRequest}; +use devlaunch_core::clients::{herdr_resume_line, herdr_resume_lines}; use devlaunch_core::domain::config; use devlaunch_core::domain::locks::LockError; use devlaunch_core::domain::metadata; @@ -40,8 +41,8 @@ use devlaunch_core::flows::kill::{ Marker, NoSignal, Released, Standing, StillHeld, Sweep, TableUnreadable, }; use devlaunch_core::flows::launch::{ - BranchNotNamed, ClaudeProfileProblem, ColdRefused, LaunchAborted, LaunchNotice, LaunchRefusal, - NotPrepared, SessionRefused, + BranchNotNamed, ClaudeProfileProblem, ColdRefused, DerivedImageKept, LaunchAborted, + LaunchNotice, LaunchRefusal, NotPrepared, SessionRefused, }; use devlaunch_core::flows::lifecycle::{ Insistence, KeptBecause, LifecycleNotice, LockKeptBecause, NotAdopted, Promotion, PrunePlan, @@ -55,6 +56,7 @@ use devlaunch_core::flows::migration::{Listing, MigrationReport}; use devlaunch_core::flows::provision::{BundleFailed, FailureLevel, ProvisionEvent}; use devlaunch_core::flows::pull_request::Refusal as PullRequestRefusal; use devlaunch_core::flows::records::{RecordsNotice, StartupError}; +use devlaunch_core::flows::refresh_stale::{AgentState, Planned, Skip, Tally, Verdict}; use devlaunch_core::flows::repo_manager::{ CacheNotice, Cleanup, CloneError, EnsureRepoError, NotRefreshed, Refusal, RefusalReason, RemoveTreeError, WrongRepoLock, @@ -321,6 +323,97 @@ pub(crate) fn stale_image_notes<'a>( .collect() } +/// What `dl --refresh-stale` says when no workspace is stale. +pub(crate) const REFRESH_STALE_NOTHING: &str = + "No workspace runs an older image than its reference names."; + +/// The one line `dl --refresh-stale` says about one stale workspace, before it +/// acts: that it will recreate it, or that it leaves it alone and why. +pub(crate) fn refresh_stale_line(planned: &Planned) -> String { + let Planned { + workspace_id: id, + reference, + verdict, + } = planned; + match verdict { + Verdict::Refresh => { + format!("Refreshing {id}: it runs an older image than {reference} now names.") + } + Verdict::Skip(Skip::Stopped) => format!( + "Skipping {id}: it is stopped, and a refresh starts nothing. \ + `dl {id} recreate` starts it on the new image." + ), + Verdict::Skip(skip) => format!( + "Skipping {id}: {}. Run `dl {id} recreate` once it is done.", + skip_reason(skip) + ), + } +} + +/// The line `dl --refresh-stale` says when a workspace its plan would refresh +/// is judged again just before its recreate, and is now to be left alone. +pub(crate) fn refresh_stale_rejudged_line(id: &str, skip: &Skip) -> String { + format!( + "Skipping {id} after all, judged again before its recreate: {}. \ + Run `dl {id} recreate` once it is done.", + skip_reason(skip) + ) +} + +/// Why `dl --refresh-stale` leaves a stale workspace alone, as a clause. +fn skip_reason(skip: &Skip) -> String { + match skip { + Skip::LaunchUnderWay => "another dl is launching it".to_owned(), + Skip::Stopped => "it is stopped".to_owned(), + Skip::LockUnreadable { why } => format!("could not ask its launch lock ({why})"), + Skip::ProcessesUnread { why } => { + format!("could not read the processes in its container ({why})") + } + Skip::Building { program, pid } => format!("a build runs in it ({program}, pid {pid})"), + Skip::AgentRunsACommand { program, pid } => { + format!("an agent runs a command in it ({program}, pid {pid})") + } + Skip::SessionsUnseen => "an agent runs in it, and this dl is not in a herdr pane, so it \ + cannot see the sessions to start them again" + .to_owned(), + Skip::HerdrUnanswered => "herdr did not say which panes it has".to_owned(), + Skip::Unresumable { pane_id } => { + format!("the agent in pane {pane_id} has no saved line to start it again") + } + Skip::AgentBusy { pane_id, state } => match state { + AgentState::Working => format!("the agent in pane {pane_id} is working"), + AgentState::Blocked => { + format!("the agent in pane {pane_id} waits on a permission prompt") + } + AgentState::Unknown => { + format!("herdr does not know what the agent in pane {pane_id} is doing") + } + AgentState::Unanswered => { + format!("herdr did not say what the agent in pane {pane_id} is doing") + } + }, + Skip::SessionsOutsidePanes { running, held } => format!( + "{running} agent sessions run in it and herdr's panes hold {held}, so the others \ + would end with nothing to start them again" + ), + } +} + +/// The last line of `dl --refresh-stale`. +pub(crate) fn refresh_stale_summary(tally: Tally) -> String { + let Tally { + refreshed, + skipped, + failed, + } = tally; + let noun = if refreshed == 1 { + "workspace" + } else { + "workspaces" + }; + format!("Refreshed {refreshed} stale {noun}, skipped {skipped}, and {failed} failed.") +} + /// The lines `dl --ls` writes under its table for the repositories whose last /// background sweep left something to say. /// @@ -3269,13 +3362,13 @@ pub(crate) fn launch_notice(notice: &LaunchNotice) -> Option { // --- the agents a recreate ends (devlaunch#673; no Python line) LaunchNotice::AgentSessionsUnseen { workspace_id } => format!( "This dl is not in a herdr pane, so it cannot see the agents running in \ - {workspace_id}. The recreate ends them. Run 'claude --resume' in the new \ - container to pick a conversation up again." + {workspace_id}. The recreate ends them. {}", + every_agents_resume() ), LaunchNotice::AgentSessionsUnanswered { workspace_id } => format!( "herdr did not say which panes it has, so the agents running in {workspace_id} \ - end with the recreate. Run 'claude --resume' in the new container to pick a \ - conversation up again." + end with the recreate. {}", + every_agents_resume() ), LaunchNotice::AgentSessionsHeld { workspace_id, @@ -3296,23 +3389,40 @@ pub(crate) fn launch_notice(notice: &LaunchNotice) -> Option { workspace_id, pane_id, agent, - } if agent == "claude" => format!( - "Pane {pane_id} runs an agent in {workspace_id}, and herdr saved no line to start \ - it again. The recreate ends it. Run 'claude --resume' there to pick the \ - conversation up again." - ), - LaunchNotice::AgentSessionUnresumable { - workspace_id, - pane_id, - agent, - } => format!( - "Pane {pane_id} runs {agent} in {workspace_id}, and herdr saved no line to start \ - it again. The recreate ends it. Start {agent} there again to pick the \ - conversation up." - ), + } => { + let what_to_do = match herdr_resume_line(agent) { + Some(resume) => format!("Run '{resume}' there to pick the conversation up again."), + None => format!("Start {agent} there again to pick the conversation up."), + }; + format!( + "Pane {pane_id} runs {agent} in {workspace_id}, and herdr saved no line to start \ + it again. The recreate ends it. {what_to_do}" + ) + } LaunchNotice::AgentSessionRestarted { pane_id } => { format!("Started the agent in pane {pane_id} again.") } + LaunchNotice::DerivedImageRemoved { + workspace_id, + image, + } => format!( + "{workspace_id} runs an older image. Removed the tag {image} that devpod built it \ + under, so the recreate builds the image again from the newer one." + ), + LaunchNotice::DerivedImageKept(DerivedImageKept { + workspace_id, + image, + why, + }) => derived_image_kept(workspace_id, image, why), + LaunchNotice::AgentSessionLineNotKept { pane_id } => format!( + "Started the agent in pane {pane_id} again, and herdr did not keep the line that \ + starts it. The next recreate ends it." + ), + LaunchNotice::AgentSessionLineUnchecked { pane_id } => format!( + "Started the agent in pane {pane_id} again, and could not read herdr's session \ + file to check that herdr kept the line that starts it. If it did not, the next \ + recreate ends it." + ), LaunchNotice::AgentSessionNotRestarted { pane_id, line } => format!( "Could not start the agent in pane {pane_id} again. Run this in that pane: {line}" ), @@ -3557,6 +3667,34 @@ pub(crate) fn is_binary_missing(aborted: &LaunchAborted) -> bool { } } +/// What to run to pick up a conversation an unseen recreate ended, for each agent +/// dl knows: dl cannot say which agent a pane ran, so it names every agent's own +/// resume rather than claude's alone. +fn every_agents_resume() -> String { + let quoted: Vec = herdr_resume_lines() + .iter() + .map(|line| format!("'{line}'")) + .collect(); + let listed = match quoted.split_last() { + Some((last, rest)) if !rest.is_empty() => format!("{} or {last}", rest.join(", ")), + _ => quoted.concat(), + }; + format!( + "To pick a conversation up again, run its agent's resume in the new container: \ + {listed}." + ) +} + +/// devpod's tag for a stale workspace's image stayed, so a recreate keeps the +/// older image. +fn derived_image_kept(workspace_id: &str, image: &str, why: &str) -> String { + format!( + "{workspace_id} runs an older image, and dl could not remove the tag {image} that \ + devpod built it under ({why}). A recreate builds the container from the older image \ + again. Remove the tag with 'docker image rm --force {image}', then recreate." + ) +} + /// Why a launch will not go ahead, in the words `_run_cli` refused it with. /// /// `None` is a refusal with nothing to say: `devpod up` and `devpod stop` write @@ -3579,6 +3717,14 @@ pub(crate) fn launch_refusal(refused: &LaunchRefusal) -> Option { &format!("{owner}/{repo}@{branch}"), &format!("{recorded_owner}/{recorded_repo}@{recorded_branch}"), )), + LaunchRefusal::DerivedImageKept(DerivedImageKept { + workspace_id, + image, + why, + }) => Some(format!( + "{} The refresh does not recreate it.", + derived_image_kept(workspace_id, image, why) + )), LaunchRefusal::BranchNotNamed { owner, repo, error } => Some(format!( "Repository '{owner}/{repo}': {}", branch_not_named(error) @@ -5166,8 +5312,8 @@ mod tests { assert!(here.ends_with(&format!(": {line}")), "{here}"); } - /// devlaunch#673. A pane herdr says runs codex is not told to run - /// 'claude --resume'; claude's own pane keeps the words it had. + /// devlaunch#673. A pane herdr says runs codex is told codex's own resume, not + /// 'claude --resume'; an agent dl does not know is told to start it again. #[test] fn a_pane_with_no_line_is_told_about_the_agent_it_runs() { let unresumable = |agent: &str| { @@ -5179,13 +5325,41 @@ mod tests { .expect("a sentence") }; let codex = unresumable("codex"); - assert!(codex.contains("codex"), "{codex}"); + assert!(codex.contains("Run 'codex resume' there"), "{codex}"); assert!(!codex.contains("claude"), "{codex}"); assert_eq!( unresumable("claude"), - "Pane w1:p2 runs an agent in myws, and herdr saved no line to start it again. \ + "Pane w1:p2 runs claude in myws, and herdr saved no line to start it again. \ The recreate ends it. Run 'claude --resume' there to pick the conversation up again." ); + assert_eq!( + unresumable("aider"), + "Pane w1:p2 runs aider in myws, and herdr saved no line to start it again. \ + The recreate ends it. Start aider there again to pick the conversation up." + ); + } + + /// devlaunch#673. When dl cannot see which agents a recreate ends, it does not + /// tell a codex or gemini user to run 'claude --resume': it names each known + /// agent's own resume. + #[test] + fn agents_dl_cannot_see_are_told_every_known_agents_resume() { + for said in [ + LaunchNotice::AgentSessionsUnseen { + workspace_id: "myws".to_owned(), + }, + LaunchNotice::AgentSessionsUnanswered { + workspace_id: "myws".to_owned(), + }, + ] + .iter() + .map(|notice| launch_notice(notice).expect("a sentence")) + { + assert!(!said.contains("Run 'claude --resume'"), "{said}"); + for resume in ["'claude --resume'", "'codex resume'", "'gemini --resume'"] { + assert!(said.contains(resume), "{resume} missing: {said}"); + } + } } #[test] @@ -5208,8 +5382,9 @@ mod tests { .as_deref(), Some( "This dl is not in a herdr pane, so it cannot see the agents running in myws. \ - The recreate ends them. Run 'claude --resume' in the new container to pick a \ - conversation up again." + The recreate ends them. To pick a conversation up again, run its agent's \ + resume in the new container: 'claude --resume', 'codex resume' or \ + 'gemini --resume'." ) ); assert_eq!( @@ -5219,8 +5394,8 @@ mod tests { .as_deref(), Some( "herdr did not say which panes it has, so the agents running in myws end with \ - the recreate. Run 'claude --resume' in the new container to pick a \ - conversation up again." + the recreate. To pick a conversation up again, run its agent's resume in the \ + new container: 'claude --resume', 'codex resume' or 'gemini --resume'." ) ); assert_eq!( @@ -5231,7 +5406,7 @@ mod tests { }) .as_deref(), Some( - "Pane w1:p2 runs an agent in myws, and herdr saved no line to start it again. \ + "Pane w1:p2 runs claude in myws, and herdr saved no line to start it again. \ The recreate ends it. Run 'claude --resume' there to pick the conversation up \ again." ) @@ -5245,7 +5420,8 @@ mod tests { .as_deref(), Some( "Pane w1:p2 runs codex in myws, and herdr saved no line to start it again. \ - The recreate ends it. Start codex there again to pick the conversation up." + The recreate ends it. Run 'codex resume' there to pick the conversation up \ + again." ) ); assert_eq!( @@ -5255,6 +5431,17 @@ mod tests { .as_deref(), Some("Started the agent in pane w1:p1 again.") ); + assert_eq!( + launch_notice(&LaunchNotice::AgentSessionLineUnchecked { + pane_id: "w3:pAX".to_owned(), + }) + .as_deref(), + Some( + "Started the agent in pane w3:pAX again, and could not read herdr's session \ + file to check that herdr kept the line that starts it. If it did not, the \ + next recreate ends it." + ) + ); } #[test] @@ -6976,4 +7163,105 @@ mod tests { )); assert!(line.contains("owner/repo@#579"), "{line}"); } + + // --- dl --refresh-stale (devlaunch#673) + + fn planned(verdict: Verdict) -> Planned { + Planned { + workspace_id: "myws".to_owned(), + reference: "ghcr.io/o/img:latest".to_owned(), + verdict, + } + } + + #[test] + fn the_refresh_plan_names_the_workspace_and_what_it_will_do() { + assert_eq!( + refresh_stale_line(&planned(Verdict::Refresh)), + "Refreshing myws: it runs an older image than ghcr.io/o/img:latest now names." + ); + assert_eq!( + refresh_stale_line(&planned(Verdict::Skip(Skip::Building { + program: "cc1plus".to_owned(), + pid: 77, + }))), + "Skipping myws: a build runs in it (cc1plus, pid 77). Run `dl myws recreate` once it is done." + ); + } + + /// Every skip says why in words of its own, so a reader can tell two apart. + #[test] + fn each_reason_to_skip_reads_differently() { + let reasons = [ + Skip::LaunchUnderWay, + Skip::LockUnreadable { + why: "eacces".to_owned(), + }, + Skip::ProcessesUnread { + why: "not running".to_owned(), + }, + Skip::Building { + program: "make".to_owned(), + pid: 1, + }, + Skip::AgentRunsACommand { + program: "bash".to_owned(), + pid: 1, + }, + Skip::SessionsUnseen, + Skip::HerdrUnanswered, + Skip::Unresumable { + pane_id: "w1:p1".to_owned(), + }, + Skip::AgentBusy { + pane_id: "w1:p1".to_owned(), + state: AgentState::Working, + }, + Skip::AgentBusy { + pane_id: "w1:p1".to_owned(), + state: AgentState::Blocked, + }, + Skip::AgentBusy { + pane_id: "w1:p1".to_owned(), + state: AgentState::Unknown, + }, + Skip::AgentBusy { + pane_id: "w1:p1".to_owned(), + state: AgentState::Unanswered, + }, + Skip::SessionsOutsidePanes { + running: 2, + held: 1, + }, + ]; + let lines: Vec = reasons + .into_iter() + .map(|skip| refresh_stale_line(&planned(Verdict::Skip(skip)))) + .collect(); + for line in &lines { + assert!(line.starts_with("Skipping myws: "), "{line}"); + } + let distinct: std::collections::BTreeSet<&String> = lines.iter().collect(); + assert_eq!(distinct.len(), lines.len(), "{lines:#?}"); + } + + #[test] + fn the_refresh_summary_counts_what_happened() { + assert_eq!( + refresh_stale_summary(Tally { + refreshed: 2, + skipped: 1, + failed: 1 + }), + "Refreshed 2 stale workspaces, skipped 1, and 1 failed." + ); + assert_eq!( + refresh_stale_summary(Tally { + refreshed: 1, + skipped: 0, + failed: 0 + }), + "Refreshed 1 stale workspace, skipped 0, and 0 failed." + ); + } } diff --git a/rust/dl/tests/read_side.rs b/rust/dl/tests/read_side.rs index bc7b4b0a..ff1b1c8c 100644 --- a/rust/dl/tests/read_side.rs +++ b/rust/dl/tests/read_side.rs @@ -90,6 +90,15 @@ impl World { } fn dl_with(&self, args: &[&str], extra: &[(&str, &str)]) -> Run { + let output = self + .command(args, extra) + .output() + .expect("the dl binary runs"); + Run::of(&output, &self.root) + } + + /// The `dl` command [`World::dl_with`] runs, before it runs. + fn command(&self, args: &[&str], extra: &[(&str, &str)]) -> Command { let root = self.root.display().to_string(); let mut command = Command::new(env!("CARGO_BIN_EXE_dl")); command @@ -117,8 +126,7 @@ impl World { for (name, value) in extra { command.env(name, value); } - let output = command.output().expect("the dl binary runs"); - Run::of(&output, &self.root) + command } fn path(&self, relative: &str) -> PathBuf { @@ -415,7 +423,7 @@ fn world_with_a_stale_workspace() -> World { std::fs::write( records.join("workspace_result.json"), format!( - r#"{{"ContainerDetails": {{"Id": "c1", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"# + r#"{{"ContainerDetails": {{"ID": "c1", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"# ), ) .expect("a create result"); @@ -916,3 +924,230 @@ fn a_verb_with_no_workspace_opens_the_selector_and_no_terminal_picks_nothing() { assert_eq!(run.err, "", "dl {args:?} acted on a workspace"); } } + +// =========================================================================== +// --refresh-stale (devlaunch#673) +// =========================================================================== + +/// The git-sourced workspace, the second one these tests make stale. +const SECOND_ID: &str = "devpod-upstream"; + +/// What a quiet container's processes read as: one `sleep`. +const QUIET: &str = r"printf '1 (sleep) S 0 1 1 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 1000 1000 10\nsleep\037infinity\ndevlaunch-end\n'"; +/// What a container with a build in it reads as: a `cc1plus` beside the `sleep`. +const BUILDING: &str = r"printf '1 (sleep) S 0 1 1 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 1000 1000 10\nsleep\037infinity\n77 (cc1plus) R 1 1 1 0 -1 4194560 155 0 0 0 0 0 0 0 20 0 1 0 1000 1000 10\ncc1plus\ndevlaunch-end\n'"; + +/// The full scenario with each `(workspace, container, answer)` stale: its +/// container created from [`STALE_REFERENCE`], and a docker that says the +/// reference has since moved. +/// +/// The n-th `docker exec` into a container runs the shell in +/// `exec/.` when that file exists, and `answer` otherwise. Every +/// docker call is logged to `docker.log`. The devpod in front of the shim copies +/// what `dl` has printed so far to `stdout-at-up-` when it is asked +/// to `up` that workspace, which is what [`World::refresh_stale`] reads the +/// order of events from. +fn world_with_stale(stale: &[(&str, &str, &str)]) -> World { + let world = World::full(); + let root = world.root.display().to_string(); + std::fs::create_dir_all(world.path("exec")).expect("an exec directory"); + for (id, container, answer) in stale { + let records = world.path(&format!("devpod/contexts/default/workspaces/{id}")); + std::fs::create_dir_all(&records).expect("a devpod record directory"); + std::fs::write( + records.join("workspace.json"), + format!(r#"{{"id": "{id}"}}"#), + ) + .expect("a devpod record"); + std::fs::write( + records.join("workspace_result.json"), + format!( + r#"{{"ContainerDetails": {{"ID": "{container}", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"# + ), + ) + .expect("a create result"); + std::fs::write(world.path(&format!("exec/{container}")), answer).expect("an answer"); + } + let docker = world.path("bin/docker"); + std::fs::write( + &docker, + format!( + r#"#!/bin/sh +echo "$*" >> "{root}/docker.log" +case "$1" in +inspect) + kind=$3; shift 5 + for name in "$@"; do + case "$kind $name" in + "image sha256:old") echo 'sha256:old 2026-10-01T00:00:00Z ["sha256:l1"]' ;; + "image {STALE_REFERENCE}") echo 'sha256:new 2026-10-05T00:00:00.123Z ["sha256:l2"]' ;; + container\ *) [ -f "{root}/exec/$name" ] && echo "$name sha256:old" ;; + esac + done ;; +exec) + n=$(( $(cat "{root}/exec/$4.count" 2>/dev/null || echo 0) + 1 )) + echo "$n" > "{root}/exec/$4.count" + if [ -f "{root}/exec/$4.$n" ]; then . "{root}/exec/$4.$n"; else . "{root}/exec/$4"; fi ;; +*) exit 1 ;; +esac +"# + ), + ) + .expect("a fake docker"); + make_executable(&docker); + let devpod = world.path("bin/devpod"); + let shim = std::fs::read_to_string(&devpod).expect("the scenario's devpod"); + let (shebang, rest) = shim.split_once('\n').expect("a shebang line"); + std::fs::write( + &devpod, + format!( + "{shebang}\n[ \"$1\" = up ] && cp \"{root}/stdout.txt\" \"{root}/stdout-at-up-$2\"\n{rest}" + ), + ) + .expect("a devpod that notes the order"); + world +} + +impl World { + /// `dl --refresh-stale`, its stdout written to `stdout.txt` as it goes and + /// its devpod calls logged to `devpod.log`. + fn refresh_stale(&self) -> Run { + let log = self.path("devpod.log").display().to_string(); + let config = self.path("shim-config.json").display().to_string(); + let stdout = std::fs::File::create(self.path("stdout.txt")).expect("a stdout file"); + let output = self + .command( + &["--refresh-stale"], + &[("DEVPOD_SHIM_LOG", &log), ("DEVPOD_SHIM_CONFIG", &config)], + ) + .stdout(stdout) + .output() + .expect("the dl binary runs"); + let mut run = Run::of(&output, &self.root); + run.out = self.read("stdout.txt"); + run + } + + /// The workspaces devpod was asked to `up`, in order. + fn upped(&self) -> Vec { + self.read("devpod.log") + .lines() + .filter_map(|line| { + let argv: Vec = serde_json::from_str::(line) + .ok()? + .get("argv")? + .as_array()? + .iter() + .filter_map(|word| word.as_str().map(str::to_owned)) + .collect(); + (argv.first()? == "up").then(|| argv.get(1).cloned())? + }) + .collect() + } +} + +/// The plan line for a stale workspace `dl --refresh-stale` will recreate. +fn refreshing(id: &str) -> String { + format!("Refreshing {id}: it runs an older image than {STALE_REFERENCE} now names.\n") +} + +#[test] +fn a_workspace_that_got_busy_after_the_plan_is_not_recreated() { + // Both are quiet when the plan is read. By the time the first recreate is + // done, a build runs in the second: the plan said refresh it, and it is + // judged again before its recreate and left alone. + let world = world_with_stale(&[(STALE_ID, "c1", QUIET), (SECOND_ID, "c2", QUIET)]); + std::fs::write(world.path("exec/c2.2"), BUILDING).expect("a later answer"); + let run = world.refresh_stale(); + run.succeeded(); + assert_eq!(world.upped(), vec![STALE_ID.to_owned()]); + assert_eq!( + run.out, + format!( + "{}{}Skipping {SECOND_ID} after all, judged again before its recreate: a build \ + runs in it (cc1plus, pid 77). Run `dl {SECOND_ID} recreate` once it is done.\n\ + Refreshed 1 stale workspace, skipped 1, and 0 failed.\n", + refreshing(STALE_ID), + refreshing(SECOND_ID), + ) + ); +} + +#[test] +fn a_refresh_with_nothing_stale_says_so_and_reads_no_container() { + let world = world_with_stale(&[]); + let run = world.refresh_stale(); + run.succeeded(); + assert_eq!( + run.out, + "No workspace runs an older image than its reference names.\n" + ); + assert!( + !world.read("docker.log").contains("exec"), + "docker exec ran: {}", + world.read("docker.log") + ); + assert_eq!(world.upped(), Vec::::new()); +} + +#[test] +fn a_refresh_prints_its_plan_then_recreates_the_quiet_one_and_skips_the_build() { + let world = world_with_stale(&[(STALE_ID, "c1", QUIET), (SECOND_ID, "c2", BUILDING)]); + let run = world.refresh_stale(); + run.succeeded(); + let plan = format!( + "{}Skipping {SECOND_ID}: a build runs in it (cc1plus, pid 77). \ + Run `dl {SECOND_ID} recreate` once it is done.\n", + refreshing(STALE_ID) + ); + assert_eq!(world.read(&format!("stdout-at-up-{STALE_ID}")), plan); + assert_eq!(world.upped(), vec![STALE_ID.to_owned()]); + assert_eq!( + run.out, + format!("{plan}Refreshed 1 stale workspace, skipped 1, and 0 failed.\n") + ); +} + +#[test] +fn a_refresh_whose_recreate_fails_goes_on_to_the_next_and_exits_1() { + let world = world_with_stale(&[(STALE_ID, "c1", QUIET), (SECOND_ID, "c2", QUIET)]); + std::fs::write( + world.path("shim-config.json"), + format!( + r#"{{"responses": [{{"prefix": ["up", "{STALE_ID}"], "returncode": 1, "stdout": "", "stderr": "up failed"}}]}}"# + ), + ) + .expect("a shim config"); + let run = world.refresh_stale(); + run.exited(1); + assert_eq!( + world.upped(), + vec![STALE_ID.to_owned(), SECOND_ID.to_owned()] + ); + assert!( + run.out + .ends_with("Refreshed 1 stale workspace, skipped 0, and 1 failed.\n"), + "stdout: {}", + run.out + ); +} + +#[test] +fn a_refresh_skips_a_stopped_stale_workspace() { + let world = world_with_stale(&[( + STALE_ID, + "c1", + "echo 'Error response from daemon: container c1 is not running' >&2; exit 1", + )]); + let run = world.refresh_stale(); + run.succeeded(); + assert_eq!(world.upped(), Vec::::new()); + assert_eq!( + run.out, + format!( + "Skipping {STALE_ID}: it is stopped, and a refresh starts nothing. \ + `dl {STALE_ID} recreate` starts it on the new image.\n\ + Refreshed 0 stale workspaces, skipped 1, and 0 failed.\n" + ) + ); +} diff --git a/scripts/public-api-snapshots.sh b/scripts/public-api-snapshots.sh index f9e0afd4..a15846d7 100755 --- a/scripts/public-api-snapshots.sh +++ b/scripts/public-api-snapshots.sh @@ -37,7 +37,7 @@ # The limit that is left, and it is not one type. A type `api` never re-exports # but a promised signature names is reachable from outside and is classified as # binary surface, so a break in it diffs public-api.rest.txt alone. Measured on -# the checked-in files rather than guessed at: 40 such types own close to six +# the checked-in files rather than guessed at: 41 such types own close to six # hundred rows over there. `--print-residual` lists them, needs no toolchain, # and prints the exact row count; the type count above is the figure # `test/test_public_api_snapshots_doc.py` diffs against it, because that is the @@ -55,7 +55,7 @@ # # What that means for reading a rest-file diff: it is routine for a row whose # subject is nothing a promised signature names, and a contract change for a row -# whose subject is one of the 40. `--print-residual` is how you tell. +# whose subject is one of the 41. `--print-residual` is how you tell. # # Whether the tool does this already, since the obvious first question is why # any of it is hand-rolled (#352 asked it explicitly). It does not, in the pin diff --git a/test/fixtures/rust_source.py b/test/fixtures/rust_source.py index 49d9ae8f..5a012010 100644 --- a/test/fixtures/rust_source.py +++ b/test/fixtures/rust_source.py @@ -180,6 +180,36 @@ def aid_agent_names() -> Tuple[str, ...]: return names +def core_agent_resumes() -> Tuple[Tuple[str, str], ...]: + """Each agent and the words that resume it, as core's `AGENT_RESUMES` lists them.""" + source = _read(HERDR_RS) + match = re.search(r"const AGENT_RESUMES: &\[\(&str, &str\)\] = &\[(.*?)\];", source, re.DOTALL) + assert match, f"{HERDR_RS.name} no longer declares `const AGENT_RESUMES`" + rows = tuple(re.findall(r'\("([^"]+)",\s*"([^"]+)"\)', match.group(1))) + assert rows, f"{HERDR_RS.name} declares AGENT_RESUMES with no rows in it" + return rows + + +def aid_agent_resumes() -> Tuple[Tuple[str, str], ...]: + """Each row of aid's `AGENTS` table and its `resume` words, joined by a space.""" + source = _read(AID_REWRITE_RS) + start = source.find("const AGENTS: &[(&str, Agent)] = &[") + assert start != -1, f"{AID_REWRITE_RS.name} no longer declares `const AGENTS`" + end = source.find("\n];", start) + assert end != -1, f"`const AGENTS` in {AID_REWRITE_RS.name} is not closed at column zero" + rows = [] + for name, body in re.findall( + r'^\s{8}"([^"]+)",$(.*?)(?=^\s{8}"[^"]+",$|\Z)', + source[start:end], + re.MULTILINE | re.DOTALL, + ): + resume = re.search(r"^\s+resume: &\[([^\]]*)\],$", body, re.MULTILINE) + assert resume, f"aid's row for {name} has no `resume`" + rows.append((name, " ".join(re.findall(r'"([^"]+)"', resume.group(1))))) + assert rows, f"{AID_REWRITE_RS.name} declares AGENTS with no agent names in it" + return tuple(rows) + + def core_session_manager_tab_var() -> str: """The variable the pane shell reads to learn which tab it was spawned in. diff --git a/test/unit/test_session_manager.py b/test/unit/test_session_manager.py index 20c5ef7b..7fd3ce25 100644 --- a/test/unit/test_session_manager.py +++ b/test/unit/test_session_manager.py @@ -62,6 +62,20 @@ def test_neither_launcher_knows_an_agent_the_other_does_not(self): ) +# The words after each agent's program that pick its last conversation up again. +RESUMES = (("claude", "--resume"), ("codex", "resume"), ("gemini", "--resume")) + + +class TestTheResumeBothTablesKnow: + """dl tells a user how to resume an agent a recreate ended; aid resumes it.""" + + def test_core_resumes_each_agent_in_its_own_words(self): + assert rust_source.core_agent_resumes() == RESUMES + + def test_aid_resumes_each_agent_in_the_same_words(self): + assert rust_source.aid_agent_resumes() == RESUMES + + @pytest.mark.parametrize("name", AGENTS) def test_an_agent_name_is_a_bare_program_name(name): """A path or an argument in the list would never match what dl reads.