Merge remote-tracking branch 'origin/main' into blitz/codex-structured-sessions
# Conflicts: # crates/maestro-daemon/tests/worktree.rs # crates/maestro-e2e/tests/e2e.rs # web/src/attach.ts # web/src/connections.test.ts # web/src/connections.ts
This commit is contained in:
Generated
+1
@@ -1467,6 +1467,7 @@ dependencies = [
|
||||
"maestro-cli",
|
||||
"maestro-daemon",
|
||||
"maestro-tui",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"tracing-subscriber",
|
||||
]
|
||||
|
||||
@@ -405,15 +405,17 @@ fn run_git(dir: &std::path::Path, args: &[&str]) {
|
||||
);
|
||||
}
|
||||
|
||||
/// 49ab7e9: a new-branch add onto a name that already exists suffixes the
|
||||
/// branch (and so the directory) instead of failing, per docs/WORKTREES.md.
|
||||
/// `git worktree add -b` alone fails outright when the branch already exists,
|
||||
/// even with the target directory free — a random handle commonly collides
|
||||
/// with one an earlier, already-removed worktree left behind. `add_new`
|
||||
/// sidesteps it on the branch name itself instead of erroring (#77).
|
||||
#[test]
|
||||
fn add_new_suffixes_when_branch_already_exists() {
|
||||
fn add_new_suffixes_branch_name_on_branch_collision() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo = repo_dir.path().to_path_buf();
|
||||
init_repo(&repo);
|
||||
|
||||
// pre-create the branch
|
||||
// pre-create the branch, unused by any worktree.
|
||||
let status = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&repo)
|
||||
@@ -425,9 +427,17 @@ fn add_new_suffixes_when_branch_already_exists() {
|
||||
let wt_root = TempDir::new().unwrap();
|
||||
let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
|
||||
|
||||
let path = mgr.add_new(&repo, "feat/dup").unwrap();
|
||||
let path = mgr
|
||||
.add_new(&repo, "feat/dup")
|
||||
.expect("sidesteps the branch collision");
|
||||
assert!(path.ends_with("feat/dup-2"), "{}", path.display());
|
||||
assert!(path.is_dir());
|
||||
assert!(path.exists());
|
||||
|
||||
// The pre-existing branch is untouched; the new worktree checked out the
|
||||
// suffixed one instead of erroring or adopting `feat/dup`.
|
||||
let branches = worktree_branches(&repo);
|
||||
assert!(branches.contains(&"feat/dup-2".to_string()), "{branches:?}");
|
||||
assert!(!branches.contains(&"feat/dup".to_string()), "{branches:?}");
|
||||
}
|
||||
|
||||
/// The project picker lists linked worktrees as pickable rows, so `base_repo`
|
||||
|
||||
@@ -383,13 +383,11 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
/// #66: a second spawn onto an *existing* branch that already has a worktree
|
||||
/// is rejected with a structured worktree error; the first worktree stays
|
||||
/// intact. (A second `create_branch: true` spawn is a different case: 49ab7e9
|
||||
/// suffixes the new branch name instead, see
|
||||
/// `colliding_branch_name_gets_suffixed_too`.)
|
||||
/// #66/#77: a second spawn onto the same branch used to be rejected outright;
|
||||
/// since the branch-collision suffixing landed, it succeeds instead — the
|
||||
/// second worktree lands on `<branch>-2` and the first stays untouched.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn second_spawn_on_same_branch_is_rejected() {
|
||||
async fn second_spawn_on_same_branch_gets_suffixed() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
@@ -399,28 +397,33 @@ async fn second_spawn_on_same_branch_is_rejected() {
|
||||
.spawn_session(&worktree_req(base.path(), "feature/dup", true))
|
||||
.await
|
||||
.expect("first spawn");
|
||||
let dup = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/dup", false))
|
||||
.await;
|
||||
match dup {
|
||||
Err(maestro_e2e::ClientError::Api(maestro_protocol::ClientError::Api { info, .. })) => {
|
||||
assert_eq!(
|
||||
info.code, "worktree_failed",
|
||||
"expected worktree error: {info:?}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected rejection for duplicate branch, got {other:?}"),
|
||||
}
|
||||
let second = client
|
||||
.spawn_session(&worktree_req(base.path(), "feature/dup", true))
|
||||
.await
|
||||
.expect("second spawn suffixes instead of rejecting");
|
||||
|
||||
// Exactly one registration, one directory, still usable.
|
||||
// Count branch records only: the worktree path now contains the branch
|
||||
// name too (`worktrees/feature/dup`), so a substring match over every line
|
||||
// would count the same tree twice.
|
||||
let holders = git_worktree_list(base.path())
|
||||
.iter()
|
||||
.filter(|w| w.as_str() == "branch refs/heads/feature/dup")
|
||||
.count();
|
||||
assert_eq!(holders, 1, "duplicate spawn must not register another tree");
|
||||
let suffixed = base.path().join("worktrees").join("feature").join("dup-2");
|
||||
assert_eq!(
|
||||
second.cwd, suffixed,
|
||||
"second worktree must land on the suffixed path"
|
||||
);
|
||||
assert!(suffixed.is_dir());
|
||||
|
||||
// Both registrations exist: the original branch, untouched, and the
|
||||
// suffixed one the second spawn created.
|
||||
let registrations = git_worktree_list(base.path());
|
||||
assert!(
|
||||
registrations
|
||||
.iter()
|
||||
.any(|l| l.as_str() == "branch refs/heads/feature/dup"),
|
||||
"original branch must stay registered: {registrations:?}"
|
||||
);
|
||||
assert!(
|
||||
registrations
|
||||
.iter()
|
||||
.any(|l| l.as_str() == "branch refs/heads/feature/dup-2"),
|
||||
"suffixed branch must be registered too: {registrations:?}"
|
||||
);
|
||||
assert!(
|
||||
base.path()
|
||||
.join("worktrees")
|
||||
@@ -431,7 +434,8 @@ async fn second_spawn_on_same_branch_is_rejected() {
|
||||
);
|
||||
|
||||
let _origin = throwaway_origin(base.path());
|
||||
client.kill_session(first.id).await.expect("kill");
|
||||
client.kill_session(first.id).await.expect("kill first");
|
||||
client.kill_session(second.id).await.expect("kill second");
|
||||
}
|
||||
|
||||
/// #66/#56: when the client picks a *linked worktree* as base_repo, placement
|
||||
|
||||
@@ -29,3 +29,6 @@ tracing-subscriber.workspace = true
|
||||
maestro-cli = { path = "../maestro-cli", optional = true, default-features = false }
|
||||
maestro-daemon = { path = "../maestro-daemon", optional = true, default-features = false }
|
||||
maestro-tui = { path = "../maestro-tui", optional = true }
|
||||
|
||||
[dev-dependencies]
|
||||
tempfile.workspace = true
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
//! Process-level routing for the umbrella `maestro` binary (#10): each
|
||||
//! enabled top-level subcommand must reach its own crate's runtime dispatch,
|
||||
//! not just parse successfully.
|
||||
//!
|
||||
//! A malformed `--config` file makes every crate's dispatch fail identically
|
||||
//! at config load, before any subcommand touches tmux, the network, or a
|
||||
//! terminal. That lets the assertions tell "clap parsed it but the handler
|
||||
//! never ran" (a usage error, exit 2) apart from "it dispatched into the real
|
||||
//! crate" (a `toml decode` runtime error via `main`'s `maestro: {e}` printer,
|
||||
//! exit 1).
|
||||
//!
|
||||
//! The absence tests below only compile when the matching feature is off, so
|
||||
//! they run under CI's per-feature-subset matrix (`--no-default-features
|
||||
//! --features cli|daemon|tui`) rather than the default all-features build.
|
||||
|
||||
use std::io::Write;
|
||||
use std::path::PathBuf;
|
||||
use std::process::{Command, Output};
|
||||
|
||||
fn bin() -> PathBuf {
|
||||
PathBuf::from(env!("CARGO_BIN_EXE_maestro"))
|
||||
}
|
||||
|
||||
fn bad_config() -> tempfile::NamedTempFile {
|
||||
let mut f = tempfile::NamedTempFile::new().expect("create temp config file");
|
||||
write!(f, "this is not valid toml {{{{{{").expect("write temp config");
|
||||
f
|
||||
}
|
||||
|
||||
fn run(args: &[&str]) -> Output {
|
||||
Command::new(bin())
|
||||
.args(args)
|
||||
.output()
|
||||
.unwrap_or_else(|e| panic!("spawn `maestro {args:?}`: {e}"))
|
||||
}
|
||||
|
||||
/// Real dispatch happened: exit 1, `main`'s `"maestro: {e}"` prefix, and the
|
||||
/// config crate's error surfacing through — never clap's own usage-error
|
||||
/// page, which would mean the subcommand never reached its handler.
|
||||
fn assert_dispatched(out: &Output) {
|
||||
let stderr = String::from_utf8_lossy(&out.stderr);
|
||||
assert_eq!(out.status.code(), Some(1), "stderr:\n{stderr}");
|
||||
assert!(stderr.starts_with("maestro: "), "stderr:\n{stderr}");
|
||||
assert!(stderr.contains("toml decode"), "stderr:\n{stderr}");
|
||||
assert!(!stderr.contains("Usage:"), "stderr:\n{stderr}");
|
||||
}
|
||||
|
||||
/// Never reached a handler: clap rejected the subcommand outright because the
|
||||
/// feature that defines it is off. Only used by the `not(feature = "...")`
|
||||
/// tests below, which themselves only exist in a single-feature build.
|
||||
#[cfg(not(all(feature = "cli", feature = "daemon", feature = "tui")))]
|
||||
fn assert_unrecognized(out: &Output, name: &str) {
|
||||
let stderr = String::from_utf8_lossy(&out.stderr);
|
||||
assert_eq!(out.status.code(), Some(2), "stderr:\n{stderr}");
|
||||
assert!(
|
||||
stderr.contains("unrecognized subcommand"),
|
||||
"stderr:\n{stderr}"
|
||||
);
|
||||
assert!(stderr.contains(name), "stderr:\n{stderr}");
|
||||
}
|
||||
|
||||
#[cfg(feature = "cli")]
|
||||
#[test]
|
||||
fn list_dispatches_into_maestro_cli() {
|
||||
let cfg = bad_config();
|
||||
let out = run(&["list", "--config", cfg.path().to_str().unwrap()]);
|
||||
assert_dispatched(&out);
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "cli"))]
|
||||
#[test]
|
||||
fn list_is_unrecognized_without_cli_feature() {
|
||||
assert_unrecognized(&run(&["list"]), "list");
|
||||
}
|
||||
|
||||
#[cfg(feature = "daemon")]
|
||||
#[test]
|
||||
fn daemon_dispatches_into_maestro_daemon() {
|
||||
let cfg = bad_config();
|
||||
let out = run(&["daemon", "--config", cfg.path().to_str().unwrap()]);
|
||||
assert_dispatched(&out);
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "daemon"))]
|
||||
#[test]
|
||||
fn daemon_is_unrecognized_without_daemon_feature() {
|
||||
assert_unrecognized(&run(&["daemon"]), "daemon");
|
||||
}
|
||||
|
||||
#[cfg(feature = "tui")]
|
||||
#[test]
|
||||
fn tui_dispatches_into_maestro_tui() {
|
||||
let cfg = bad_config();
|
||||
let out = run(&["tui", "--config", cfg.path().to_str().unwrap()]);
|
||||
assert_dispatched(&out);
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "tui"))]
|
||||
#[test]
|
||||
fn tui_is_unrecognized_without_tui_feature() {
|
||||
assert_unrecognized(&run(&["tui"]), "tui");
|
||||
}
|
||||
+21
-2
@@ -64,6 +64,7 @@ export function createAttachConnection(
|
||||
<div class="attach-body"></div>
|
||||
`;
|
||||
|
||||
const titleEl = el.querySelector(".attach-title") as HTMLSpanElement;
|
||||
const body = el.querySelector(".attach-body") as HTMLDivElement;
|
||||
const stateEl = el.querySelector(".attach-state") as HTMLSpanElement;
|
||||
const disconnectBtn = el.querySelector(".disconnect") as HTMLButtonElement;
|
||||
@@ -323,6 +324,9 @@ export function createAttachConnection(
|
||||
let ws: WebSocket;
|
||||
let reconnectMs = INITIAL_RECONNECT_MS;
|
||||
let reconnectTimer: number | null = null;
|
||||
// Snapshot replay starts at row zero, which can leave Codex at its oldest line.
|
||||
// Follow it once per socket; later scrolling remains under the operator's control.
|
||||
let awaitingReplay = true;
|
||||
|
||||
const sendControl = (msg: ClientControl) => {
|
||||
if (ws.readyState !== WebSocket.OPEN) return;
|
||||
@@ -342,6 +346,7 @@ export function createAttachConnection(
|
||||
};
|
||||
|
||||
function connect(): void {
|
||||
awaitingReplay = true;
|
||||
ws = new WebSocket(attachWsUrl(sessionId));
|
||||
ws.binaryType = "arraybuffer";
|
||||
|
||||
@@ -362,10 +367,20 @@ export function createAttachConnection(
|
||||
return;
|
||||
}
|
||||
if (ev.data instanceof ArrayBuffer) {
|
||||
term.write(new Uint8Array(ev.data));
|
||||
const replay = awaitingReplay;
|
||||
awaitingReplay = false;
|
||||
term.write(new Uint8Array(ev.data), () => {
|
||||
if (replay) term.scrollToBottom();
|
||||
});
|
||||
} else if (ev.data instanceof Blob) {
|
||||
// Some browsers may default to Blob despite binaryType="arraybuffer".
|
||||
ev.data.arrayBuffer().then((b) => term.write(new Uint8Array(b)));
|
||||
void ev.data.arrayBuffer().then((b) => {
|
||||
const replay = awaitingReplay;
|
||||
awaitingReplay = false;
|
||||
term.write(new Uint8Array(b), () => {
|
||||
if (replay) term.scrollToBottom();
|
||||
});
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -463,6 +478,10 @@ export function createAttachConnection(
|
||||
// The closure above: the same bytes-out path keystrokes and the mobile key
|
||||
// bar already take.
|
||||
sendInput,
|
||||
updateInfo(next: AttachInfo) {
|
||||
titleEl.innerHTML = paneTitleHtml(sessionId, next);
|
||||
titleEl.title = paneTitleAttr(sessionId, next);
|
||||
},
|
||||
dispose() {
|
||||
if (disposed) return;
|
||||
disposed = true;
|
||||
|
||||
@@ -27,6 +27,7 @@ function fakeConn(kind: string, id: string) {
|
||||
sendInput(data: string) {
|
||||
sent.push({ id, data });
|
||||
},
|
||||
updateInfo() {},
|
||||
dispose() {
|
||||
disposed.push(`${kind}:${id}`);
|
||||
},
|
||||
|
||||
@@ -110,6 +110,7 @@ export class ConnectionPool {
|
||||
} else {
|
||||
this.cache.delete(id);
|
||||
this.cache.set(id, conn); // LRU refresh
|
||||
if (info) conn.updateInfo(info);
|
||||
}
|
||||
} else {
|
||||
conn = this.create(id, info);
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
// transcript (structured.ts).
|
||||
|
||||
import type { Uuid } from "./types.js";
|
||||
import type { AttachInfo } from "./paneTitle.js";
|
||||
|
||||
export interface PaneConnection {
|
||||
readonly sessionId: Uuid;
|
||||
@@ -21,6 +22,8 @@ export interface PaneConnection {
|
||||
* literal-byte escape hatch, docs/COMMAND-BAR.md). A structured connection
|
||||
* has no PTY, so this is a no-op there. */
|
||||
sendInput(data: string): void;
|
||||
/** Refresh the pane header after the catalog renames a live session. */
|
||||
updateInfo(info: AttachInfo): void;
|
||||
/** Fully disconnect and remove the element. */
|
||||
dispose(): void;
|
||||
}
|
||||
|
||||
@@ -114,6 +114,7 @@ export function createStructuredConnection(
|
||||
`;
|
||||
|
||||
const stateEl = el.querySelector(".attach-state") as HTMLSpanElement;
|
||||
const titleEl = el.querySelector(".attach-title") as HTMLSpanElement;
|
||||
const configEl = el.querySelector(".structured-config") as HTMLDivElement;
|
||||
const configTrigger = el.querySelector(".structured-config-trigger") as HTMLButtonElement;
|
||||
const configMenu = el.querySelector(".structured-config-menu") as HTMLDivElement;
|
||||
@@ -985,6 +986,10 @@ export function createStructuredConnection(
|
||||
// No PTY behind a structured session — the leader's literal-byte
|
||||
// escape hatch has nothing to do here.
|
||||
},
|
||||
updateInfo(next: AttachInfo) {
|
||||
titleEl.innerHTML = paneTitleHtml(sessionId, next);
|
||||
titleEl.title = paneTitleAttr(sessionId, next);
|
||||
},
|
||||
dispose() {
|
||||
if (disposed) return;
|
||||
disposed = true;
|
||||
|
||||
Reference in New Issue
Block a user