Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions crates/orbyn-core/src/project/outside_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,17 +271,20 @@ fn read_target(target: &Path, fence: &Fence) -> Result<OutsideFile> {
// A folder is never listed from here.
return Err(refused("That path is not a file."));
}
// Before any path is read back from the open file: a file with several names has no one path, and the
// system may hand back any of them (macOS reports whichever name it has cached), so the path checks below
// could refuse it for a different reason from one run to the next.
if link_count(&file).map_or(true, |count| count != 1) {
return Err(refused(
"That file has more than one name, so Orbyn can't tell where it lives.",
));
}
let opened = crate::project::files::opened_path(&file)
.map_err(|_| refused("That file can't be checked."))?;
if !within(&opened, target) || !within(target, &opened) {
return Err(refused("That file moved while it was opened."));
}
fence.check(&opened)?;
if link_count(&file).map_or(true, |count| count != 1) {
return Err(refused(
"That file has more than one name, so Orbyn can't tell where it lives.",
));
}
let size = meta.len();
let path = target.to_string_lossy().into_owned();
let mut bytes = Vec::new();
Expand Down Expand Up @@ -468,8 +471,25 @@ mod tests {
std::fs::write(fixture.root.join("data/service.json"), "token").unwrap();
let alias = fixture.root.join("elsewhere/copy.json");
std::fs::hard_link(fixture.root.join("data/service.json"), &alias).unwrap();
let error = read(&fixture, &alias).unwrap_err();
assert!(error.to_string().contains("more than one name"), "{error}");
// The system may report either name for the open file; the answer must not depend on which.
for _ in 0..50 {
let error = read(&fixture, &alias).unwrap_err();
assert!(error.to_string().contains("more than one name"), "{error}");
}
}

#[cfg(unix)]
#[test]
fn a_second_name_for_a_file_outside_the_fence_is_refused_too() {
let fixture = fixture();
let first = fixture.root.join("elsewhere/one.txt");
std::fs::write(&first, "text").unwrap();
let second = fixture.root.join("elsewhere/two.txt");
std::fs::hard_link(&first, &second).unwrap();
for name in [&first, &second] {
let error = read(&fixture, name).unwrap_err();
assert!(error.to_string().contains("more than one name"), "{error}");
}
}

#[cfg(target_os = "macos")]
Expand Down
3 changes: 3 additions & 0 deletions web/e2e/real/preview-input-trace.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ test("page evaluation and denied input cannot publish an agent pointer", async (
});

test("link navigation and same-URL reload clear the cursor and allow later legitimate clicks", async ({ page }, info) => {
// Quarantined: about one run in three the tab's title stays on the old document after the click. See
// https://github.com/orbyn-dev/orbyn-engineering/issues/339 for the evidence and what to check.
test.fixme(true, "Flaky in CI: issue #339");
await withPreview(page, info, async ({ base, cues, act }) => {
for (const selector of ["#link", "#reload"]) {
const served = loads;
Expand Down
2 changes: 1 addition & 1 deletion web/e2e/real/worktree-cleanup-scopes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { startProject } from "./service";

const TITLE = "Automatic worktree cleanup";

test("switching cleanup on captures the inherited legacy age for the selected project", async ({ page }) => {
test("shows the inherited legacy age for the selected project, and On after Off starts at 7", async ({ page }) => {
const { service, projectId } = await startProject({ fakeEngines: true });
try {
await service.api("PUT", "/api/preferences", { values: { storage_cleanup: { worktree_settled_after_days: 12 }, worktree_cleanup: {} } });
Expand Down
26 changes: 18 additions & 8 deletions web/src/app/details.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { act, render, screen, waitFor, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { beforeEach, describe, expect, it } from "vitest";
import { installMockFetch, setViewportWidth } from "../test-support";
import { useUi } from "../state/ui";
import { api } from "../api/client";
import { useWorkspace } from "../state/workspace";
import { useUi } from "../state/ui";
import { App } from "./App";

beforeEach(() => {
Expand Down Expand Up @@ -139,15 +140,24 @@ describe("the Details button and the details card", () => {
expect(await screen.findByRole("dialog", { name: /Delete “/ })).toBeInTheDocument();
});

it("replaces the composer with the read-only banner once the thread is archived", async () => {
it("shows the read-only banner for an archived thread", async () => {
setViewportWidth(1400);
const user = userEvent.setup();
// Archived on the service itself, so nothing the app reads again can undo it mid-test.
await api.patchThread("t-done", { archived: true });
render(<App />);
await user.click(await details());
expect(await screen.findByRole("textbox", { name: /^Message / })).toBeInTheDocument();
const thread = useWorkspace.getState().threads["t-done"]!;
act(() => useWorkspace.getState().putThread({ ...thread, lifecycle: { ...thread.lifecycle!, archived_at: "2026-11-03T09:00:00Z" } }));
expect(await screen.findByText("This thread is archived and read-only.")).toBeInTheDocument();
expect(await screen.findByText("This thread is archived and read-only.", {}, wait)).toBeInTheDocument();
expect(screen.queryByRole("textbox", { name: /^Message / })).toBeNull();
});

it("switches the composer to the banner when the open thread is archived", async () => {
setViewportWidth(1400);
render(<App />);
expect(await screen.findByRole("textbox", { name: /^Message / }, wait)).toBeInTheDocument();
await api.patchThread("t-done", { archived: true });
await act(async () => {
await useWorkspace.getState().refreshThread("t-done");
});
expect(await screen.findByText("This thread is archived and read-only.", {}, wait)).toBeInTheDocument();
expect(screen.queryByRole("textbox", { name: /^Message / })).toBeNull();
});
});
71 changes: 71 additions & 0 deletions web/src/features/settings/NativeSessions.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { act, render, screen } from "@testing-library/react";
import { afterEach, beforeEach, expect, it, vi } from "vitest";
import { ApiFailure, request } from "../../api/client";
import type { EngineStatus } from "../../api/types";
import { NativeSessions } from "./NativeSessions";

vi.mock("../../api/client", async (original) => ({ ...(await original<object>()), request: vi.fn() }));

const engine = { id: "agent", tier: "acp", installed: true } as unknown as EngineStatus;
const caps = { can_list: true, can_load: true, can_resume: false, can_delete: true };

beforeEach(() => {
vi.useFakeTimers();
vi.mocked(request).mockReset();
});
afterEach(() => vi.useRealTimers());

it("asks again when the agent turns the capability read away as busy, and then offers the sessions", async () => {
vi.mocked(request)
.mockRejectedValueOnce(new ApiFailure("engine_busy", "The agent is busy.", 409))
.mockResolvedValue(caps as never);
render(<NativeSessions engine={engine} />);
expect(screen.queryByText("Native sessions")).toBeNull();
await act(async () => {
await vi.advanceTimersByTimeAsync(1000);
});
expect(vi.mocked(request)).toHaveBeenCalledTimes(2);
expect(screen.getByText("Native sessions")).toBeInTheDocument();
});

it("says so, with a Retry, when the agent is still busy after the last try, and Retry asks again", async () => {
vi.mocked(request).mockRejectedValue(new ApiFailure("engine_busy", "The agent is busy.", 409));
render(<NativeSessions engine={engine} />);
await act(async () => {
await vi.advanceTimersByTimeAsync(60_000);
});
expect(vi.mocked(request)).toHaveBeenCalledTimes(7);
expect(screen.getByRole("alert")).toHaveTextContent("Couldn’t check saved sessions");
expect(screen.getByRole("alert")).toHaveTextContent("The agent is busy. Try again in a moment.");
vi.mocked(request).mockResolvedValue(caps as never);
await act(async () => {
screen.getByRole("button", { name: "Retry" }).click();
await vi.advanceTimersByTimeAsync(0);
});
expect(screen.getByText("Native sessions")).toBeInTheDocument();
expect(screen.queryByRole("alert")).toBeNull();
});

it.each([
["engine_unavailable", 503],
["unsupported", 400],
["unauthorized", 401],
])("does not ask again for a final answer (%s)", async (code, status) => {
vi.mocked(request).mockRejectedValue(new ApiFailure(code, "No.", status));
render(<NativeSessions engine={engine} />);
await act(async () => {
await vi.advanceTimersByTimeAsync(60_000);
});
expect(vi.mocked(request)).toHaveBeenCalledTimes(1);
expect(screen.queryByText("Native sessions")).toBeNull();
expect(screen.queryByRole("alert")).toBeNull();
});

it("does not ask again after a network failure", async () => {
vi.mocked(request).mockRejectedValue(new TypeError("Failed to fetch"));
render(<NativeSessions engine={engine} />);
await act(async () => {
await vi.advanceTimersByTimeAsync(60_000);
});
expect(vi.mocked(request)).toHaveBeenCalledTimes(1);
});
48 changes: 42 additions & 6 deletions web/src/features/settings/NativeSessions.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useEffect, useRef, useState } from "react";
import { request } from "../../api/client";
import { ApiFailure, request } from "../../api/client";
import type { EngineStatus } from "../../api/types";
import type { AcpNativeSession } from "../../generated/AcpNativeSession";
import type { AcpNativeSessionImported } from "../../generated/AcpNativeSessionImported";
Expand All @@ -10,8 +10,13 @@ import { useUi } from "../../state/ui";
import { useWorkspace } from "../../state/workspace";
import { Button } from "../../ui/controls";
import { ConfirmDialog } from "../../ui/Dialog";
import { ErrorState } from "../../ui/states";
import { SettingRow } from "../../ui/SettingRow";

/** A capability read turned away as busy is asked again up to this many times, a little later each time. */
const CAPABILITY_RETRIES = 6;
const CAPABILITY_RETRY_MS = 400;

const base = (engine: string) => `/api/engines/${encodeURIComponent(engine)}/native-sessions`;

/** An ACP agent's own saved conversations, offered only when it says it can list them. */
Expand All @@ -23,21 +28,51 @@ export function NativeSessions({ engine }: { engine: EngineStatus }) {
const [sessions, setSessions] = useState<AcpNativeSession[]>([]);
const [next, setNext] = useState<string | null>(null);
const [busy, setBusy] = useState<string | null>(null);
const [stuck, setStuck] = useState<ApiFailure | null>(null);
const [again, setAgain] = useState(0);
const [deleting, setDeleting] = useState<AcpNativeSession | null>(null);
const epoch = useRef(0);
const active = engine.tier === "acp" && engine.installed;
const project = projectId ?? projects[0]?.id ?? null;

useEffect(() => {
epoch.current += 1;
setCaps(null); setSessions([]); setNext(null); setBusy(null);
setCaps(null); setSessions([]); setNext(null); setBusy(null); setStuck(null);
if (!active) return;
const abort = new AbortController();
request<AcpSessionCapabilities>("GET", `${base(engine.id)}/capabilities`, undefined, abort.signal).then(setCaps, () => undefined);
return () => abort.abort();
}, [engine.id, active]);
let timer: ReturnType<typeof setTimeout> | undefined;
let retries = 0;
// The agent answers one thing at a time, so a second ask while the first is still being answered (the page asking
// again, an update running) is turned away as busy. That is not "cannot list sessions": ask again shortly. Any
// other answer (disabled, unsupported, not signed in, a start-up error) is final, and asking again would not change it.
const ask = () =>
request<AcpSessionCapabilities>("GET", `${base(engine.id)}/capabilities`, undefined, abort.signal).then(setCaps, (failure: unknown) => {
if (abort.signal.aborted || !(failure instanceof ApiFailure) || failure.code !== "engine_busy") return;
if (retries >= CAPABILITY_RETRIES) {
// Still busy after the last try: say so, rather than look like an agent that cannot list sessions.
setStuck(failure);
return;
}
retries += 1;
timer = setTimeout(ask, CAPABILITY_RETRY_MS * retries);
});
void ask();
return () => {
abort.abort();
clearTimeout(timer);
};
}, [engine.id, active, again]);

if (!active || !caps?.can_list) return null;
// One line for the person, whatever words the service used; the row sits with whatever else the section shows.
const stuckRow = active && stuck ? (
<ErrorState
compact
title="Couldn’t check saved sessions"
failure={new ApiFailure(stuck.code, "The agent is busy. Try again in a moment.", stuck.status)}
onRetry={() => setAgain((n) => n + 1)}
/>
) : null;
if (!active || !caps?.can_list) return stuckRow;
const canImport = caps.can_load || caps.can_resume;
const fail = (title: string, failure: unknown) => toast(`${title}: ${toFailure(failure).message}`);

Expand Down Expand Up @@ -91,6 +126,7 @@ export function NativeSessions({ engine }: { engine: EngineStatus }) {

return (
<div className="native-sessions">
{stuckRow}
<SettingRow title="Native sessions" description={projects.length === 0 ? "Add a project before importing sessions." : "Resume agent-owned conversations as threads."}>
{projects.length > 0 ? (
<>
Expand Down
14 changes: 12 additions & 2 deletions web/src/features/settings/WorktreeCleanup.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { beforeEach, expect, it, vi } from "vitest";
import { request } from "../../api/client";
import { WorktreeCleanup } from "./WorktreeCleanup";
import { daysWhenTurnedOn, WorktreeCleanup } from "./WorktreeCleanup";
import { scopedFixture } from "./scopedPreferenceFixture.test-support";

vi.mock("../../api/client", async (original) => ({ ...(await original<object>()), request: vi.fn() }));
Expand Down Expand Up @@ -33,6 +33,16 @@ it("shows the saved days beside On, saves a new number on leaving it, and Off hi
expect(days).toHaveValue(14);
await user.clear(days); await user.type(days, "3"); await user.tab();
await waitFor(() => sent({ mode: "custom", rules: { settled_after_days: 3 } }));
await user.selectOptions(screen.getByRole("combobox", { name: "Automatic worktree cleanup" }), "off");
// The controls rest while the save and the read after it are under way; a change made then is not taken.
const select = screen.getByRole("combobox", { name: "Automatic worktree cleanup" });
await waitFor(() => expect(select).toBeEnabled());
await user.selectOptions(select, "off");
await waitFor(() => sent({ mode: "off" }));
});

it("turns on from Off at seven days whatever the field still says, and keeps a set number otherwise", () => {
expect(daysWhenTurnedOn(null, "12")).toBe(7);
expect(daysWhenTurnedOn(null, "")).toBe(7);
expect(daysWhenTurnedOn(14, "30")).toBe(30);
expect(daysWhenTurnedOn(14, "")).toBe(7);
});
9 changes: 8 additions & 1 deletion web/src/features/settings/WorktreeCleanup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ import { useUi } from "../../state/ui";
const DEFAULT_DAYS = 7;
const TITLE = "Automatic worktree cleanup";

/**
* The days a worktree stays when cleanup is turned on. Off has no days, so from Off it is the default, never the
* field's text: that text is reset only after the screen has caught up with Off, and a change can land before it.
*/
export const daysWhenTurnedOn = (current: number | null, draft: string): number =>
current === null ? DEFAULT_DAYS : Number(draft) || DEFAULT_DAYS;

type Policy = { mode?: string; rules?: { settled_after_days?: unknown } };
const policy = (value: unknown): Policy => value !== null && typeof value === "object" ? value as Policy : {};

Expand All @@ -33,7 +40,7 @@ function CleanupSelect({ label, current, disabled, mixed, set }: { label: string
};
return <span className="set-inline-control">
<select aria-label={label} disabled={disabled} value={mixed ? "mixed" : current === null ? "off" : "on"}
onChange={(event) => set(event.target.value === "on" ? Number(draft) || DEFAULT_DAYS : null)}>
onChange={(event) => set(event.target.value === "on" ? daysWhenTurnedOn(current, draft) : null)}>
{mixed ? <option value="mixed" disabled>Mixed</option> : null}
<option value="off">Off</option><option value="on">On</option>
</select>
Expand Down
Loading