mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-14 00:00:58 +00:00
Persist Always-allow grants with the session
Grants were in-memory only — every restart re-asked for approved tools/commands. Saved on the session record, re-applied on engine rebuild.
This commit is contained in:
@@ -30,6 +30,16 @@ def _load_roots(raw: Optional[str]) -> list[dict]:
|
|||||||
return value if isinstance(value, list) else []
|
return value if isinstance(value, list) else []
|
||||||
|
|
||||||
|
|
||||||
|
def _load_grants(raw: Optional[str]) -> dict:
|
||||||
|
if not raw:
|
||||||
|
return {}
|
||||||
|
try:
|
||||||
|
value = json.loads(raw)
|
||||||
|
except json.JSONDecodeError:
|
||||||
|
return {}
|
||||||
|
return value if isinstance(value, dict) else {}
|
||||||
|
|
||||||
|
|
||||||
def _display_title(row: sqlite3.Row) -> Optional[str]:
|
def _display_title(row: sqlite3.Row) -> Optional[str]:
|
||||||
"""Title precedence for every read path: a manual rename (renamed=1) always wins,
|
"""Title precedence for every read path: a manual rename (renamed=1) always wins,
|
||||||
then the generated auto_title, then the first-line snapshot `save()` wrote."""
|
then the generated auto_title, then the first-line snapshot `save()` wrote."""
|
||||||
@@ -84,6 +94,7 @@ class ConversationStore:
|
|||||||
"ALTER TABLE sessions ADD COLUMN origin_label TEXT",
|
"ALTER TABLE sessions ADD COLUMN origin_label TEXT",
|
||||||
"ALTER TABLE sessions ADD COLUMN auto_title TEXT",
|
"ALTER TABLE sessions ADD COLUMN auto_title TEXT",
|
||||||
"ALTER TABLE sessions ADD COLUMN renamed INTEGER DEFAULT 0",
|
"ALTER TABLE sessions ADD COLUMN renamed INTEGER DEFAULT 0",
|
||||||
|
"ALTER TABLE sessions ADD COLUMN grants TEXT",
|
||||||
):
|
):
|
||||||
try:
|
try:
|
||||||
self._conn.execute(ddl)
|
self._conn.execute(ddl)
|
||||||
@@ -180,13 +191,13 @@ class ConversationStore:
|
|||||||
title = record.title or title_from(record.messages)
|
title = record.title or title_from(record.messages)
|
||||||
self._conn.execute(
|
self._conn.execute(
|
||||||
"""
|
"""
|
||||||
INSERT INTO sessions (session_id, workspace, model, mode, title, agent, n_msgs, messages, extra_roots, updated_at)
|
INSERT INTO sessions (session_id, workspace, model, mode, title, agent, n_msgs, messages, extra_roots, grants, updated_at)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, NULL, ?, CURRENT_TIMESTAMP)
|
VALUES (?, ?, ?, ?, ?, ?, ?, NULL, ?, ?, CURRENT_TIMESTAMP)
|
||||||
ON CONFLICT(session_id) DO UPDATE SET
|
ON CONFLICT(session_id) DO UPDATE SET
|
||||||
workspace = excluded.workspace, model = excluded.model, mode = excluded.mode,
|
workspace = excluded.workspace, model = excluded.model, mode = excluded.mode,
|
||||||
title = COALESCE(sessions.title, excluded.title), agent = excluded.agent,
|
title = COALESCE(sessions.title, excluded.title), agent = excluded.agent,
|
||||||
n_msgs = excluded.n_msgs, messages = NULL, extra_roots = excluded.extra_roots,
|
n_msgs = excluded.n_msgs, messages = NULL, extra_roots = excluded.extra_roots,
|
||||||
updated_at = CURRENT_TIMESTAMP
|
grants = excluded.grants, updated_at = CURRENT_TIMESTAMP
|
||||||
""",
|
""",
|
||||||
(
|
(
|
||||||
sid,
|
sid,
|
||||||
@@ -197,6 +208,7 @@ class ConversationStore:
|
|||||||
record.agent,
|
record.agent,
|
||||||
len(record.messages),
|
len(record.messages),
|
||||||
json.dumps(record.extra_roots or []),
|
json.dumps(record.extra_roots or []),
|
||||||
|
json.dumps(record.grants or {}),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
self._conn.commit()
|
self._conn.commit()
|
||||||
@@ -228,6 +240,7 @@ class ConversationStore:
|
|||||||
extra_roots=_load_roots(
|
extra_roots=_load_roots(
|
||||||
row["extra_roots"] if "extra_roots" in row.keys() else None
|
row["extra_roots"] if "extra_roots" in row.keys() else None
|
||||||
),
|
),
|
||||||
|
grants=_load_grants(row["grants"] if "grants" in row.keys() else None),
|
||||||
pinned=bool(row["pinned"]),
|
pinned=bool(row["pinned"]),
|
||||||
archived=bool(row["archived"]),
|
archived=bool(row["archived"]),
|
||||||
origin=row["origin"],
|
origin=row["origin"],
|
||||||
|
|||||||
@@ -85,6 +85,13 @@ _SCOPES = {s.value for s in Scope}
|
|||||||
logger = logging.getLogger("coworker.manager")
|
logger = logging.getLogger("coworker.manager")
|
||||||
|
|
||||||
|
|
||||||
|
def _grants_of(engine) -> dict[str, Any]:
|
||||||
|
"""The engine's session-scoped "Always allow" approvals, in persistable shape."""
|
||||||
|
tools = sorted(getattr(engine.permissions, "session_allow_tools", None) or ())
|
||||||
|
commands = sorted(getattr(engine.permissions, "session_allow_commands", None) or ())
|
||||||
|
return {"tools": tools, "commands": commands} if (tools or commands) else {}
|
||||||
|
|
||||||
|
|
||||||
def _approval_body(request) -> str:
|
def _approval_body(request) -> str:
|
||||||
"""Approval card body: the tool's reason (if any) plus a compact preview of its args, so a
|
"""Approval card body: the tool's reason (if any) plus a compact preview of its args, so a
|
||||||
mirrored 'Run `write_file`?' shows the path/content rather than just the tool name.
|
mirrored 'Run `write_file`?' shows the path/content rather than just the tool name.
|
||||||
@@ -384,6 +391,8 @@ class SessionManager:
|
|||||||
engine.permissions.task_rules.setdefault("send_message", set()).add(
|
engine.permissions.task_rules.setdefault("send_message", set()).add(
|
||||||
thread_target
|
thread_target
|
||||||
)
|
)
|
||||||
|
if record is not None and record.grants:
|
||||||
|
self._apply_grants(engine, record.grants)
|
||||||
self._engines[session_id] = engine
|
self._engines[session_id] = engine
|
||||||
if is_new_session:
|
if is_new_session:
|
||||||
self._emit_session_created(session_id, agent_name)
|
self._emit_session_created(session_id, agent_name)
|
||||||
@@ -2393,7 +2402,7 @@ class SessionManager:
|
|||||||
else:
|
else:
|
||||||
await self.gateway.deliver(
|
await self.gateway.deliver(
|
||||||
target,
|
target,
|
||||||
f"{body}\n(Open the app to respond.)\n[ocw:{item.id}]".strip(),
|
f"{body}\n(Open the app to respond.)\n[ow:{item.id}]".strip(),
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
@@ -2429,7 +2438,7 @@ class SessionManager:
|
|||||||
# -- inbox replies over messaging connectors --------------------------------
|
# -- inbox replies over messaging connectors --------------------------------
|
||||||
def _resolve_inbox_reply(self, event) -> bool:
|
def _resolve_inbox_reply(self, event) -> bool:
|
||||||
"""Try to handle an inbound Slack/Telegram message as an Inbox reply. Returns True if the
|
"""Try to handle an inbound Slack/Telegram message as an Inbox reply. Returns True if the
|
||||||
message carried an `[ocw:<id>]` token (so it's consumed here, not routed as a new turn) —
|
message carried an `[ow:<id>]` token (so it's consumed here, not routed as a new turn) —
|
||||||
resolving the item also releases any agent suspended on it."""
|
resolving the item also releases any agent suspended on it."""
|
||||||
from ..inbox_routing import resolve_from_reply
|
from ..inbox_routing import resolve_from_reply
|
||||||
|
|
||||||
@@ -2591,7 +2600,7 @@ class SessionManager:
|
|||||||
|
|
||||||
# -- mention router (§31) ----------------------------------------------------
|
# -- mention router (§31) ----------------------------------------------------
|
||||||
async def _route_mention(self, event, ms: MessageSource, subs) -> None:
|
async def _route_mention(self, event, ms: MessageSource, subs) -> None:
|
||||||
"""@ocw tagged in a channel. A subscribed (user-connected) coworker owns the channel
|
"""@OpenWorker tagged in a channel. A subscribed (user-connected) coworker owns the channel
|
||||||
and must answer; otherwise the per-thread coworker session handles it — spawned on the
|
and must answer; otherwise the per-thread coworker session handles it — spawned on the
|
||||||
first tag, steered by follow-ups (deduped on the thread target)."""
|
first tag, steered by follow-ups (deduped on the thread target)."""
|
||||||
from ..connectors.base import format_target
|
from ..connectors.base import format_target
|
||||||
@@ -2975,9 +2984,19 @@ class SessionManager:
|
|||||||
title=title_from(engine.messages),
|
title=title_from(engine.messages),
|
||||||
agent=getattr(engine, "agent_name", "code"),
|
agent=getattr(engine, "agent_name", "code"),
|
||||||
extra_roots=self._extra_roots_of(engine),
|
extra_roots=self._extra_roots_of(engine),
|
||||||
|
grants=_grants_of(engine),
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _apply_grants(engine: TurnEngine, grants: dict[str, Any]) -> None:
|
||||||
|
"""Re-apply a reloaded session's persisted "Always allow" approvals — they're
|
||||||
|
session-scoped, and the session outlives the process (owner-hit 2026-07-22)."""
|
||||||
|
for tool in grants.get("tools") or []:
|
||||||
|
engine.permissions.allow_tool_for_session(str(tool))
|
||||||
|
for command in grants.get("commands") or []:
|
||||||
|
engine.permissions.allow_command_for_session(str(command))
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _extra_roots_of(engine: TurnEngine) -> list[dict[str, Any]]:
|
def _extra_roots_of(engine: TurnEngine) -> list[dict[str, Any]]:
|
||||||
"""Added folders = the engine's roots minus the primary scratch (index 0)."""
|
"""Added folders = the engine's roots minus the primary scratch (index 0)."""
|
||||||
|
|||||||
@@ -24,6 +24,10 @@ class SessionRecord:
|
|||||||
# Folders added to the session beyond its primary scratch dir, each {path, writable, label}.
|
# Folders added to the session beyond its primary scratch dir, each {path, writable, label}.
|
||||||
# The primary scratch is re-provisioned at engine build, so only these extras are persisted.
|
# The primary scratch is re-provisioned at engine build, so only these extras are persisted.
|
||||||
extra_roots: list[dict[str, Any]] = field(default_factory=list)
|
extra_roots: list[dict[str, Any]] = field(default_factory=list)
|
||||||
|
# "Always allow" approvals granted in this session ({tools: [...], commands: [...]}) —
|
||||||
|
# session-scoped by design, but the session outlives the process, so they must too
|
||||||
|
# (owner-hit 2026-07-22: grants forgotten on every restart).
|
||||||
|
grants: dict[str, Any] = field(default_factory=dict)
|
||||||
pinned: bool = False
|
pinned: bool = False
|
||||||
archived: bool = False
|
archived: bool = False
|
||||||
# Where the session came from, when not user-started (§31): machine key + display label
|
# Where the session came from, when not user-started (§31): machine key + display label
|
||||||
|
|||||||
@@ -716,3 +716,38 @@ def test_provider_set_and_remove_roundtrip(tmp_path):
|
|||||||
assert not prov["zai"]["key_set_at"]
|
assert not prov["zai"]["key_set_at"]
|
||||||
|
|
||||||
assert not client.delete("/v1/providers/nope").json()["ok"]
|
assert not client.delete("/v1/providers/nope").json()["ok"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_always_allow_grants_survive_restart(tmp_path):
|
||||||
|
""""Always allow" is session-scoped, and the session outlives the process — a restart
|
||||||
|
(fresh manager over the same store) must not re-ask for an approved command
|
||||||
|
(owner-hit 2026-07-22 on the 0.1.6 walkthrough)."""
|
||||||
|
|
||||||
|
def _shell_turns():
|
||||||
|
return ScriptedProvider(
|
||||||
|
[
|
||||||
|
_tool("run_shell", {"command": "uname -a"}, call_id="c1"),
|
||||||
|
_text("done"),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
def _run_turn(client, expect_prompts):
|
||||||
|
with client.websocket_connect("/ws/session/grants1?agent=cowork") as ws:
|
||||||
|
assert ws.receive_json()["type"] == "ready"
|
||||||
|
ws.send_json({"type": "user_message", "text": "run it"})
|
||||||
|
asked = 0
|
||||||
|
while True:
|
||||||
|
ev = ws.receive_json()
|
||||||
|
if ev["type"] == "permission_required":
|
||||||
|
asked += 1
|
||||||
|
ws.send_json({"type": "approval", "decision": "always_command"})
|
||||||
|
if ev["type"] == "turn_done":
|
||||||
|
break
|
||||||
|
assert asked == expect_prompts
|
||||||
|
|
||||||
|
mgr = SessionManager(workspace=None, provider=_shell_turns())
|
||||||
|
_run_turn(TestClient(create_app(mgr)), expect_prompts=1)
|
||||||
|
|
||||||
|
# "Restart": new manager + engine rebuilt from the persisted record.
|
||||||
|
mgr2 = SessionManager(workspace=None, provider=_shell_turns())
|
||||||
|
_run_turn(TestClient(create_app(mgr2)), expect_prompts=0)
|
||||||
|
|||||||
Reference in New Issue
Block a user