mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-01 15:36:08 +00:00
Root promotion: request_directory gains primary; granted folder can become the workspace
One-way, once, scratch-primary sessions only; consent card says workspace. Live roots + shell repoint, persisted; engine rebuilds fully anchored after the turn.
This commit is contained in:
@@ -1210,6 +1210,10 @@ class TurnEngine:
|
||||
"reason": str(args.get("reason", "")),
|
||||
"path": str(args.get("path", "")),
|
||||
"writable": bool(args.get("writable", False)),
|
||||
# Root promotion (workspace-scratch-design.md §5): the agent asks for
|
||||
# the folder to become the session's primary workspace — the consent
|
||||
# card must say so, it's a different grant than a plain extra root.
|
||||
"primary": bool(args.get("primary", False)),
|
||||
},
|
||||
)
|
||||
self._audit(
|
||||
|
||||
@@ -2121,6 +2121,7 @@ def create_app(manager: SessionManager) -> FastAPI:
|
||||
data={
|
||||
"path": str(args.get("path", "")),
|
||||
"writable": bool(args.get("writable", False)),
|
||||
"primary": bool(args.get("primary", False)),
|
||||
},
|
||||
tool_call_id=tool_call_id,
|
||||
)
|
||||
@@ -2137,6 +2138,39 @@ def create_app(manager: SessionManager) -> FastAPI:
|
||||
if not path:
|
||||
return {"granted": False, "error": "no directory was provided"}
|
||||
writable = bool(resp.get("writable", args.get("writable", False)))
|
||||
if bool(args.get("primary", False)):
|
||||
# Root promotion (workspace-scratch-design.md §5) — the shell cd inside
|
||||
# is blocking, keep it off the event loop.
|
||||
promo = await asyncio.to_thread(
|
||||
manager.promote_workspace, session_id, path
|
||||
)
|
||||
if promo.get("ok"):
|
||||
return {
|
||||
"granted": True,
|
||||
"path": promo["path"],
|
||||
"writable": True,
|
||||
"primary": True,
|
||||
"note": (
|
||||
"This folder is now the session's workspace. For the rest "
|
||||
"of this turn, address it by absolute path."
|
||||
),
|
||||
}
|
||||
# Promotion refused (e.g. the session already has a workspace): still
|
||||
# honor the grant as a plain additional folder.
|
||||
res = manager.add_root(session_id, path, writable)
|
||||
if not res.get("ok"):
|
||||
return {
|
||||
"granted": False,
|
||||
"error": promo.get("error", "could not promote"),
|
||||
}
|
||||
return {
|
||||
"granted": True,
|
||||
"path": path,
|
||||
"writable": writable,
|
||||
"primary": False,
|
||||
"note": promo.get("error", "")
|
||||
+ " — granted as an additional folder instead",
|
||||
}
|
||||
res = manager.add_root(session_id, path, writable)
|
||||
if not res.get("ok"):
|
||||
return {
|
||||
|
||||
+121
-9
@@ -12,6 +12,7 @@ import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import shlex
|
||||
import shutil
|
||||
import subprocess
|
||||
import time
|
||||
@@ -160,6 +161,10 @@ class SessionManager:
|
||||
if self.default_workspace:
|
||||
self.session_store.touch_workspace(self.default_workspace)
|
||||
self._engines: dict[str, TurnEngine] = {}
|
||||
# Sessions whose workspace was promoted mid-turn (workspace-scratch-design.md §5):
|
||||
# evicted from the engine cache at the next mark_idle so the following turn
|
||||
# rebuilds fully anchored on the new workspace.
|
||||
self._promotion_rebuild: set[str] = set()
|
||||
self._running_sessions: set[str] = (
|
||||
set()
|
||||
) # sessions with an in-flight turn (busy)
|
||||
@@ -1006,6 +1011,7 @@ class SessionManager:
|
||||
data={
|
||||
"path": str(args.get("path", "")),
|
||||
"writable": bool(args.get("writable", False)),
|
||||
"primary": bool(args.get("primary", False)),
|
||||
},
|
||||
tool_call_id=tool_call_id,
|
||||
)
|
||||
@@ -1019,6 +1025,33 @@ class SessionManager:
|
||||
if not path:
|
||||
return {"granted": False, "error": "no directory was provided"}
|
||||
writable = bool(resp.get("writable", args.get("writable", False)))
|
||||
if bool(args.get("primary", False)):
|
||||
promo = await asyncio.to_thread(self.promote_workspace, session_id, path)
|
||||
if promo.get("ok"):
|
||||
return {
|
||||
"granted": True,
|
||||
"path": promo["path"],
|
||||
"writable": True,
|
||||
"primary": True,
|
||||
"note": (
|
||||
"This folder is now the session's workspace. For the rest "
|
||||
"of this turn, address it by absolute path."
|
||||
),
|
||||
}
|
||||
res = self.add_root(session_id, path, writable)
|
||||
if not res.get("ok"):
|
||||
return {
|
||||
"granted": False,
|
||||
"error": promo.get("error", "could not promote"),
|
||||
}
|
||||
return {
|
||||
"granted": True,
|
||||
"path": path,
|
||||
"writable": writable,
|
||||
"primary": False,
|
||||
"note": promo.get("error", "")
|
||||
+ " — granted as an additional folder instead",
|
||||
}
|
||||
res = self.add_root(session_id, path, writable)
|
||||
if not res.get("ok"):
|
||||
return {
|
||||
@@ -4054,6 +4087,11 @@ class SessionManager:
|
||||
or self.teams.for_worker_session(session_id)
|
||||
):
|
||||
asyncio.run_coroutine_threadsafe(self.team_tick(), self._loop)
|
||||
if session_id in self._promotion_rebuild:
|
||||
# Promotion happened this turn: drop the cached engine so the next turn
|
||||
# rebuilds with the new primary (relative anchoring, env snapshot, git).
|
||||
self._promotion_rebuild.discard(session_id)
|
||||
self._engines.pop(session_id, None)
|
||||
|
||||
def is_running(self, session_id: str) -> bool:
|
||||
return session_id in self._running_sessions
|
||||
@@ -4569,7 +4607,7 @@ class SessionManager:
|
||||
messages=engine.messages,
|
||||
title=title_from(engine.messages),
|
||||
agent=getattr(engine, "agent_name", "code"),
|
||||
extra_roots=self._extra_roots_of(engine),
|
||||
extra_roots=self._extra_roots_of(engine, session_id),
|
||||
grants=_grants_of(engine),
|
||||
compaction=(
|
||||
engine.compaction_state.as_dict()
|
||||
@@ -4590,13 +4628,23 @@ class SessionManager:
|
||||
if grants.get("readonly"):
|
||||
engine.permissions.allow_readonly_for_session()
|
||||
|
||||
@staticmethod
|
||||
def _extra_roots_of(engine: TurnEngine) -> list[dict[str, Any]]:
|
||||
"""Added folders = the engine's roots minus the primary scratch (index 0)."""
|
||||
def _extra_roots_of(
|
||||
self, engine: TurnEngine, session_id: str
|
||||
) -> list[dict[str, Any]]:
|
||||
"""User/agent-added folders = the engine's roots minus the primary (index 0) AND
|
||||
the session's provisioned scratch root. Persisting the scratch as an "extra"
|
||||
would re-add it as a plain folder on every rebuild (universal scratch made
|
||||
index-0-only slicing wrong for dual-root sessions)."""
|
||||
roots = getattr(engine, "roots", None) or []
|
||||
scratch = (self.scratch_base() / session_id).expanduser()
|
||||
try:
|
||||
scratch = scratch.resolve()
|
||||
except OSError:
|
||||
pass
|
||||
return [
|
||||
{"path": str(r.path), "writable": bool(r.writable), "label": r.label}
|
||||
for r in roots[1:]
|
||||
if r.path != scratch
|
||||
]
|
||||
|
||||
# -- LLM auto-titles (FB-010) -------------------------------------------------
|
||||
@@ -4730,15 +4778,29 @@ class SessionManager:
|
||||
else self._provision_scratch(session_id)
|
||||
)
|
||||
extra = (record.extra_roots if record else []) or []
|
||||
primary_is_scratch = self.is_temp_workspace(primary)
|
||||
out = [
|
||||
{
|
||||
"path": primary,
|
||||
"writable": True,
|
||||
"label": "scratch",
|
||||
"label": "scratch" if primary_is_scratch else "workspace",
|
||||
"primary": True,
|
||||
"exists": Path(primary).is_dir(),
|
||||
}
|
||||
]
|
||||
# Universal scratch: a real-folder session also carries its provisioned scratch
|
||||
# root (mirrors the engine-side shape so a cold read matches a live one).
|
||||
if not primary_is_scratch and self._SESSION_ID_RE.match(session_id or ""):
|
||||
scratch = self.scratch_base() / session_id
|
||||
out.append(
|
||||
{
|
||||
"path": str(scratch.expanduser().resolve()),
|
||||
"writable": True,
|
||||
"label": "scratch",
|
||||
"primary": False,
|
||||
"exists": scratch.is_dir(),
|
||||
}
|
||||
)
|
||||
for r in extra:
|
||||
p = str(r.get("path", ""))
|
||||
out.append(
|
||||
@@ -4752,6 +4814,45 @@ class SessionManager:
|
||||
)
|
||||
return out
|
||||
|
||||
def promote_workspace(self, session_id: str, path: str) -> dict[str, Any]:
|
||||
"""Root promotion (workspace-scratch-design.md §5): adopt `path` as the session's
|
||||
primary workspace. One-way and once — only while the primary is still the
|
||||
provisioned scratch; a session that already has a real workspace is never
|
||||
re-pointed. Mutates the live session (roots + shell cwd), persists, and marks
|
||||
the engine for a post-turn rebuild."""
|
||||
p = Path(path).expanduser()
|
||||
if not p.is_dir():
|
||||
return {"ok": False, "error": f"not a directory: {path}"}
|
||||
resolved = p.resolve()
|
||||
engine = self._engines.get(session_id)
|
||||
if engine is None:
|
||||
return {"ok": False, "error": "no live session to promote"}
|
||||
executor = getattr(engine, "executor", None)
|
||||
current = str(executor.cwd) if executor is not None else None
|
||||
if not current or not self.is_temp_workspace(current):
|
||||
return {"ok": False, "error": "this session already has a workspace"}
|
||||
roots = getattr(engine, "roots", None)
|
||||
if roots is None:
|
||||
return {"ok": False, "error": "this session has no directory list"}
|
||||
# Shared list: permissions, file tools, and the context injector see the new
|
||||
# primary immediately. The old scratch primary stays as the scratch root.
|
||||
roots[:] = [
|
||||
RootDir(path=resolved, writable=True, label="workspace"),
|
||||
*[r for r in roots if r.path != resolved],
|
||||
]
|
||||
try:
|
||||
# Move the live shell too — save() derives the persisted workspace from the
|
||||
# executor's cwd, so this is also what makes the promotion durable.
|
||||
res = executor.run(f"cd {shlex.quote(str(resolved))}", timeout=15)
|
||||
if res.get("exit_code") != 0:
|
||||
executor.cwd = str(resolved)
|
||||
except Exception:
|
||||
executor.cwd = str(resolved) # a respawned shell starts there
|
||||
self.save(session_id, engine)
|
||||
self.session_store.touch_workspace(str(resolved))
|
||||
self._promotion_rebuild.add(session_id)
|
||||
return {"ok": True, "path": str(resolved), "roots": self.get_roots(session_id)}
|
||||
|
||||
def add_root(
|
||||
self, session_id: str, path: str, writable: bool = False
|
||||
) -> dict[str, Any]:
|
||||
@@ -4771,7 +4872,9 @@ class SessionManager:
|
||||
r.writable = bool(writable)
|
||||
else:
|
||||
engine.roots.append(RootDir(path=resolved, writable=bool(writable)))
|
||||
self.session_store.set_extra_roots(session_id, self._extra_roots_of(engine))
|
||||
self.session_store.set_extra_roots(
|
||||
session_id, self._extra_roots_of(engine, session_id)
|
||||
)
|
||||
else:
|
||||
# A brand-new conversation has no record yet (it's only saved after the first turn) —
|
||||
# create one now so set_extra_roots has a row to update and the folder survives.
|
||||
@@ -4786,7 +4889,12 @@ class SessionManager:
|
||||
agent="cowork", # folder access is a Cowork affordance
|
||||
)
|
||||
)
|
||||
extra = [r for r in self.get_roots(session_id) if not r["primary"]]
|
||||
session_scratch = str((self.scratch_base() / session_id).expanduser().resolve())
|
||||
extra = [
|
||||
r
|
||||
for r in self.get_roots(session_id)
|
||||
if not r["primary"] and r["path"] != session_scratch
|
||||
]
|
||||
extra = [r for r in extra if Path(r["path"]).resolve() != resolved]
|
||||
extra.append(
|
||||
{
|
||||
@@ -4820,7 +4928,9 @@ class SessionManager:
|
||||
"error": "cannot remove the primary scratch directory",
|
||||
}
|
||||
engine.roots[:] = [r for r in engine.roots if r.path != resolved]
|
||||
self.session_store.set_extra_roots(session_id, self._extra_roots_of(engine))
|
||||
self.session_store.set_extra_roots(
|
||||
session_id, self._extra_roots_of(engine, session_id)
|
||||
)
|
||||
else:
|
||||
current = self.get_roots(session_id)
|
||||
if (
|
||||
@@ -4832,10 +4942,12 @@ class SessionManager:
|
||||
"ok": False,
|
||||
"error": "cannot remove the primary scratch directory",
|
||||
}
|
||||
session_scratch = (self.scratch_base() / session_id).expanduser().resolve()
|
||||
extra = [
|
||||
r
|
||||
for r in current
|
||||
if not r["primary"] and Path(r["path"]).resolve() != resolved
|
||||
if not r["primary"]
|
||||
and Path(r["path"]).resolve() not in (resolved, session_scratch)
|
||||
]
|
||||
self.session_store.set_extra_roots(
|
||||
session_id,
|
||||
|
||||
@@ -12,12 +12,17 @@ from aisuite.agents import ToolMetadata, tool
|
||||
|
||||
|
||||
def request_directory_tool() -> object:
|
||||
def request_directory(reason: str, path: str = "", writable: bool = False) -> dict:
|
||||
def request_directory(
|
||||
reason: str, path: str = "", writable: bool = False, primary: bool = False
|
||||
) -> dict:
|
||||
"""Ask the user for access to a directory when the task needs files outside the current
|
||||
ones (e.g. to read a project the user mentioned, or to save a deliverable somewhere
|
||||
specific). Explain why in `reason`; optionally suggest a `path` and whether you need
|
||||
`writable` access. The user picks/approves the folder; the result says whether it was
|
||||
granted. Do not use this to escape sandboxing — only to serve the user's request.
|
||||
`writable` access. Set `primary=true` only when the granted folder should become the
|
||||
session's main workspace (the project the whole conversation is about) — allowed once,
|
||||
and only while the session is still running on its scratch directory. The user
|
||||
picks/approves the folder; the result says whether it was granted. Do not use this to
|
||||
escape sandboxing — only to serve the user's request.
|
||||
"""
|
||||
# Real handling lives in the engine (it needs the out-of-band GUI round-trip). This body
|
||||
# only runs if no requester is wired (e.g. a headless surface).
|
||||
|
||||
@@ -751,7 +751,7 @@ export function App() {
|
||||
if (unattendedRef.current) break;
|
||||
setItems((p) => [
|
||||
...p,
|
||||
{ kind: "dirreq", reason: d.reason || "", path: d.path || "", writable: !!d.writable },
|
||||
{ kind: "dirreq", reason: d.reason || "", path: d.path || "", writable: !!d.writable, primary: !!d.primary },
|
||||
]);
|
||||
break;
|
||||
case "tool_requested":
|
||||
|
||||
@@ -26,9 +26,19 @@ export function DirectoryRequestCard({
|
||||
<div className="dirreq-card">
|
||||
<div className="dirreq-head">
|
||||
<Icon name="folderPlus" size={16} className="ico" />
|
||||
<span>The agent is requesting access to a folder</span>
|
||||
<span>
|
||||
{item.primary
|
||||
? "The agent asks to make a folder this session's workspace"
|
||||
: "The agent is requesting access to a folder"}
|
||||
</span>
|
||||
</div>
|
||||
{item.reason && <div className="dirreq-reason">“{item.reason}”</div>}
|
||||
{item.primary && (
|
||||
<div className="dirreq-reason">
|
||||
Granting makes this folder the session's primary working directory (read-write);
|
||||
the scratch directory stays available for temporary files and artifacts.
|
||||
</div>
|
||||
)}
|
||||
<div className="dirreq-pathrow">
|
||||
<input
|
||||
className="dirreq-path"
|
||||
@@ -41,16 +51,22 @@ export function DirectoryRequestCard({
|
||||
</button>
|
||||
</div>
|
||||
<div className="dirreq-actions">
|
||||
<label className="dirreq-access">
|
||||
<input type="checkbox" checked={writable} onChange={(e) => setWritable(e.target.checked)} />
|
||||
Allow writing (read-write)
|
||||
</label>
|
||||
{!item.primary && (
|
||||
<label className="dirreq-access">
|
||||
<input type="checkbox" checked={writable} onChange={(e) => setWritable(e.target.checked)} />
|
||||
Allow writing (read-write)
|
||||
</label>
|
||||
)}
|
||||
<span className="spacer" />
|
||||
<button className="btn" onClick={() => onRespond(false)}>
|
||||
Decline
|
||||
</button>
|
||||
<button className="btn primary" disabled={!path.trim()} onClick={() => onRespond(true, path.trim(), writable)}>
|
||||
Grant access
|
||||
<button
|
||||
className="btn primary"
|
||||
disabled={!path.trim()}
|
||||
onClick={() => onRespond(true, path.trim(), item.primary ? true : writable)}
|
||||
>
|
||||
{item.primary ? "Make workspace" : "Grant access"}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -378,11 +378,19 @@ export function InboxItemCard({
|
||||
onClick={() =>
|
||||
onResolve(
|
||||
item.id,
|
||||
JSON.stringify({ granted: true, path: item.data?.path || "", writable: !!item.data?.writable }),
|
||||
JSON.stringify({
|
||||
granted: true,
|
||||
path: item.data?.path || "",
|
||||
writable: item.data?.primary ? true : !!item.data?.writable,
|
||||
}),
|
||||
)
|
||||
}
|
||||
>
|
||||
{item.data?.path ? "Grant" : "Grant (no folder)"}
|
||||
{item.data?.path
|
||||
? item.data?.primary
|
||||
? "Make workspace"
|
||||
: "Grant"
|
||||
: "Grant (no folder)"}
|
||||
</button>
|
||||
<button className={BTN_BORDERED} onClick={() => onResolve(item.id, JSON.stringify({ granted: false }))}>
|
||||
Deny
|
||||
|
||||
@@ -144,6 +144,7 @@ export type Item =
|
||||
reason: string;
|
||||
path?: string;
|
||||
writable?: boolean;
|
||||
primary?: boolean; // root promotion: the folder becomes the session's workspace
|
||||
resolved?: "granted" | "denied";
|
||||
}
|
||||
| {
|
||||
|
||||
@@ -407,3 +407,73 @@ def test_gated_session_registers_request_directory(tmp_path):
|
||||
engine = mgr.get_engine("sessGated4", agent="code", workspace=str(repo))
|
||||
assert engine is not None
|
||||
assert "request_directory" in engine.registry.names()
|
||||
|
||||
|
||||
def test_gated_session_save_and_rebuild_keeps_roots_stable(tmp_path):
|
||||
"""The scratch root must never persist as an 'extra' — a save/rebuild cycle used to
|
||||
re-add it as a plain folder each time (dup roots forever)."""
|
||||
mgr = _cowork_manager(tmp_path)
|
||||
repo = _repo(tmp_path)
|
||||
sid = "sessGated5"
|
||||
engine = mgr.get_engine(sid, agent="code", workspace=str(repo))
|
||||
mgr.save(sid, engine)
|
||||
|
||||
mgr2 = _cowork_manager(tmp_path)
|
||||
engine2 = mgr2.get_engine(sid)
|
||||
assert engine2 is not None
|
||||
mgr2.save(sid, engine2)
|
||||
mgr3 = _cowork_manager(tmp_path)
|
||||
roots = mgr3.get_roots(sid)
|
||||
assert [Path(r["path"]) for r in roots] == [
|
||||
repo.resolve(),
|
||||
(mgr3.scratch_base() / sid).resolve(),
|
||||
]
|
||||
|
||||
|
||||
# -- Root promotion (workspace-scratch-design.md §5) ----------------------------
|
||||
|
||||
|
||||
def test_promote_workspace_repoints_orphan_session(tmp_path):
|
||||
mgr = _cowork_manager(tmp_path)
|
||||
repo = _repo(tmp_path, "project")
|
||||
sid = "sessPromo1"
|
||||
engine = mgr.get_engine(sid, agent="cowork")
|
||||
scratch = (mgr.scratch_base() / sid).resolve()
|
||||
|
||||
res = mgr.promote_workspace(sid, str(repo))
|
||||
assert res["ok"], res
|
||||
# Live roots re-anchored: workspace first, scratch kept.
|
||||
assert [r.path for r in engine.roots][:2] == [repo.resolve(), scratch]
|
||||
assert engine.roots[0].writable and engine.roots[0].label == "workspace"
|
||||
# Persisted: the record's workspace is the promoted folder.
|
||||
rec = mgr.session_store.load(sid)
|
||||
assert Path(rec.workspace).resolve() == repo.resolve()
|
||||
|
||||
# Post-turn rebuild (mark_idle evicts) → fully anchored dual-root session.
|
||||
mgr.mark_idle(sid)
|
||||
engine2 = mgr.get_engine(sid)
|
||||
assert engine2 is not engine
|
||||
roots = mgr.get_roots(sid)
|
||||
assert [Path(r["path"]) for r in roots] == [repo.resolve(), scratch]
|
||||
assert roots[0]["primary"] and roots[0]["label"] == "workspace"
|
||||
|
||||
|
||||
def test_promote_workspace_refused_when_session_has_one(tmp_path):
|
||||
mgr = _cowork_manager(tmp_path)
|
||||
repo = _repo(tmp_path, "projectA")
|
||||
other = _repo(tmp_path, "projectB")
|
||||
sid = "sessPromo2"
|
||||
assert mgr.get_engine(sid, agent="code", workspace=str(repo)) is not None
|
||||
res = mgr.promote_workspace(sid, str(other))
|
||||
assert not res["ok"] and "already has a workspace" in res["error"]
|
||||
|
||||
|
||||
def test_promote_workspace_is_one_way_once(tmp_path):
|
||||
mgr = _cowork_manager(tmp_path)
|
||||
a = _repo(tmp_path, "first")
|
||||
b = _repo(tmp_path, "second")
|
||||
sid = "sessPromo3"
|
||||
assert mgr.get_engine(sid, agent="cowork") is not None
|
||||
assert mgr.promote_workspace(sid, str(a))["ok"]
|
||||
res = mgr.promote_workspace(sid, str(b))
|
||||
assert not res["ok"] and "already has a workspace" in res["error"]
|
||||
|
||||
Reference in New Issue
Block a user