Compare commits

..
Author SHA1 Message Date
Miguel Palhas 2e7c5f7f58 fix: recover PWA after backgrounding
ci / web (push) Successful in 48s
ci / flake (push) Successful in 2m56s
ci / rust (push) Successful in 6m4s
2026-09-05 18:10:21 +01:00
Miguel Palhas 3529947497 Merge branch 'fix/codex-attach-scroll'
ci / web (push) Successful in 1m3s
ci / flake (push) Successful in 3m3s
ci / rust (push) Successful in 7m35s
2026-09-05 03:49:35 +01:00
Miguel Palhas 4851ad9101 fix(maestro): follow initial attach replay 2026-09-05 03:48:35 +01:00
Miguel Palhas 3cb4bebd1e fix(maestro): update second stale worktree test for branch suffixing
ci / web (push) Successful in 54s
ci / flake (push) Successful in 3m8s
ci / rust (push) Successful in 8m30s
second_spawn_on_same_branch_is_rejected asserted the same pre-#77
outright-rejection contract as the unit test fixed in 0e20baf, just at
the e2e/session level — also broken by 49ab7e9's branch-collision
suffixing and also already failing on main.

Renamed to second_spawn_on_same_branch_gets_suffixed and rewritten to
assert the actual behavior: the second spawn succeeds on a `-2`
suffixed branch and directory, and the first worktree is untouched.
2026-09-04 20:08:16 +01:00
Miguel Palhas 0e20baf7c2 fix(maestro): update stale worktree test for branch-collision suffixing
ci / web (push) Failing after 38s
ci / flake (push) Successful in 4m17s
ci / rust (push) Failing after 7m40s
add_fails_when_branch_already_exists asserted the pre-#77 behavior:
that a branch collision errors outright. Since 49ab7e9, add_new
sidesteps a branch collision with a numeric suffix instead, so the
test was failing CI on main regardless of any given change.

Replaces it with a test asserting the actual (and intended) suffixing
behavior.
2026-09-04 19:57:33 +01:00
8 changed files with 145 additions and 31 deletions
+17 -5
View File
@@ -405,13 +405,17 @@ fn run_git(dir: &std::path::Path, args: &[&str]) {
);
}
/// `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_fails_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)
@@ -423,9 +427,17 @@ fn add_fails_when_branch_already_exists() {
let wt_root = TempDir::new().unwrap();
let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
let err = mgr.add_new(&repo, "feat/dup").unwrap_err();
let msg = format!("{err}");
assert!(msg.contains("git exited"), "{msg}");
let path = mgr
.add_new(&repo, "feat/dup")
.expect("sidesteps the branch collision");
assert!(path.ends_with("feat/dup-2"), "{}", path.display());
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`
+31 -24
View File
@@ -323,10 +323,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 a branch that already has a worktree is rejected
/// with a structured worktree error; the first worktree stays intact.
/// #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();
@@ -336,28 +337,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
let second = client
.spawn_session(&worktree_req(base.path(), "feature/dup", true))
.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:?}"),
}
.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")
@@ -368,7 +374,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
+16 -2
View File
@@ -367,6 +367,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;
@@ -386,6 +389,7 @@ export function createAttachConnection(
};
function connect(): void {
awaitingReplay = true;
ws = new WebSocket(attachWsUrl(sessionId));
ws.binaryType = "arraybuffer";
@@ -406,10 +410,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();
});
});
}
};
+25
View File
@@ -130,6 +130,31 @@ describe("DaemonLink", () => {
expect(link.status().state).toBe("online");
});
it("rechecks immediately when the browser returns from the background", async () => {
const sched = manual();
let reply: DaemonInfo | null = info("aaa", "i1");
const link = new DaemonLink({
fetchInfo: async () => {
if (!reply) throw new Error("down");
return reply;
},
scheduler: sched,
reload: () => {},
});
await link.start();
reply = null;
link.noteDisconnected();
sched.run();
await Promise.resolve();
expect(link.status().state).toBe("down");
reply = info("aaa", "i1");
link.recover();
await Promise.resolve();
expect(link.status().state).toBe("online");
});
it("reloads the page when the daemon comes back on a new commit", async () => {
const sched = manual();
const reload = vi.fn();
+11
View File
@@ -148,6 +148,17 @@ export class DaemonLink {
void this.probe();
}
/**
* Recheck liveness after the browser resumes from the background. Mobile
* browsers may suspend both WebSocket events and timers while a PWA is
* hidden, leaving a stale `down` state after the daemon is already back.
*/
recover(): void {
if (this.state === "updating") return;
this.noteDisconnected();
void this.probe();
}
/**
* One `/api/info` round-trip. Resolves `true` when the daemon answered.
*
+15
View File
@@ -132,6 +132,21 @@ describe("CatalogStore reducer", () => {
});
describe("CatalogStore reconnect", () => {
it("replaces a suspended socket immediately", () => {
const first = new FakeSocket();
const second = new FakeSocket();
const { store } = storeWith([first, second]);
store.start();
store.reconnect();
expect(first.closed).toBe(true);
second.open();
// A late close from the old socket is deliberately ignored.
first.drop();
expect(store.backoff()).toBe(INITIAL_BACKOFF_MS);
});
it("doubles the delay per attempt and caps it", () => {
const sockets = Array.from({ length: 8 }, () => new FakeSocket());
const { store, scheduler } = storeWith(sockets);
+17
View File
@@ -59,6 +59,23 @@ export class CatalogStore {
this.connect();
}
/** Replace a socket that may have gone stale while the browser was suspended. */
reconnect(): void {
if (this.reconnectTimer !== null) {
this.scheduler.clearTimer(this.reconnectTimer);
this.reconnectTimer = null;
}
if (this.ws) {
// A close event from the superseded socket must not schedule a second
// connection alongside the new one.
this.ws.onclose = null;
this.ws.onerror = null;
this.ws.close();
this.ws = null;
}
this.connect();
}
/** Current reconnect delay, for tests asserting the backoff schedule. */
backoff(): number {
return this.backoffMs;
+13
View File
@@ -107,6 +107,19 @@ void link.start();
store.start();
// Installed mobile browsers suspend timers and WebSockets in the background.
// Replacing the socket on return avoids leaving the restart overlay up after a
// transient lost connection has already recovered.
function recoverFromBackground(): void {
if (document.visibilityState !== "visible") return;
link.recover();
store.reconnect();
}
document.addEventListener("visibilitychange", recoverFromBackground);
window.addEventListener("pageshow", recoverFromBackground);
window.addEventListener("online", recoverFromBackground);
// Dev-only: `?mock` seeds a representative catalog so the UI can be reviewed
// without a running daemon. Stripped from production builds by Vite.
if (import.meta.env.DEV && location.search.includes("mock")) {