diff --git a/agents/raven-code/run.py b/agents/raven-code/run.py index e349475b5..5a9909059 100644 --- a/agents/raven-code/run.py +++ b/agents/raven-code/run.py @@ -477,12 +477,19 @@ def render_config(source: Path, partition: Path, mode: str | None = None, *, una # a reply and exits, so the gate refuses every write and every command # instead of prompting (measured 2026-09-08: the model could not edit # one line and reported the task incomplete), and trunk's own one-shot - # spine names this the operator's call. The ACP hosting keeps the - # default: raven dispatching a sub-agent answers those prompts itself, - # and a person in an editor should still be asked. Builtin refusals - # (the catastrophic-command list) hold in every mode, and an explicit + # spine names this the operator's call. Builtin refusals (the + # catastrophic-command list) hold in every mode, and an explicit # permissions block in a custom config wins. config.setdefault("permissions", {}).setdefault("mode", "full") + else: + # The ACP hosting used to inherit trunk's default because that default + # was the ask tier; it has since moved to smart, where a reviewer + # speaks for that tier and lets most of it through. That is a product + # decision about raven's own surfaces, and this is not one of them: + # the person here is in an editor, watching an agent work on their + # checkout, and the prompt is how they see each write before it lands. + # Pinned rather than inherited so the tier stops moving under them. + config.setdefault("permissions", {}).setdefault("mode", "ask") # Declared, not merged: the engine composes a profile per session from the # catalogue over session/set_mode, and --mode only picks the starting entry. diff --git a/raven/config/schema.py b/raven/config/schema.py index 8e2ce124d..25980ae4f 100644 --- a/raven/config/schema.py +++ b/raven/config/schema.py @@ -1268,9 +1268,16 @@ class PermissionsConfig(Base): (``"git *"``) each mapping to a tier; several matching patterns resolve to the strictest. ``judge_model`` pins the smart-mode reviewer to one model id; empty means the running turn's own binding. + + ``smart`` out of the box. ``ask`` stopped the agent on every mutation of a + conversation, which a reader answers by reflex rather than by reading, and + a prompt answered by reflex is not a gate. Smart is not the weaker setting + it sounds like: builtin denials and user deny rules hold in every mode, the + reviewer speaks only for the ask tier, and a reviewer that cannot run + leaves the call at the same prompt ``ask`` would have shown. """ - mode: Literal["ask", "smart", "full"] = "ask" + mode: Literal["ask", "smart", "full"] = "smart" tools: dict[str, str | dict[str, str]] = Field(default_factory=dict) judge_model: str = "" judge_timeout_seconds: float = 10.0 diff --git a/raven/rpc/methods/config.py b/raven/rpc/methods/config.py index 95beeb2f0..aacc862f5 100644 --- a/raven/rpc/methods/config.py +++ b/raven/rpc/methods/config.py @@ -54,7 +54,9 @@ "tui.theme": "default", "tui.show_token_usage": True, "language": "en", - "permissions.mode": "ask", + # What config.get reports as the default and what config.unset restores, + # so it has to be the value PermissionsConfig.mode carries. + "permissions.mode": "smart", } diff --git a/raven/rpc/methods/console.py b/raven/rpc/methods/console.py index 08a1a2286..44c9a07b6 100644 --- a/raven/rpc/methods/console.py +++ b/raven/rpc/methods/console.py @@ -1048,13 +1048,13 @@ def _usage_range(params: dict): async def settings_usage(params: dict, *, agent_loop_factory=None) -> dict: """Aggregate API usage for the settings page. - LLM side reads the UsageTracker telemetry files - (``~/.raven/telemetry/usage-YYYY-MM-DD.jsonl``, one JSON row per call); - tool side counts ``tool_calls`` entries across session transcripts whose - file mtime falls inside the window. Both scans are read-only and bounded - by ``days`` (default 30, max 90). + Both halves read the same UsageTracker telemetry files + (``~/.raven/telemetry/usage-YYYY-MM-DD.jsonl``, one JSON row per call, one + per tool call), so one range means one thing across the whole reply. The + scan is read-only and bounded by ``days`` (default 30, max 90). Session + transcripts are read for their titles only. """ - from datetime import datetime, timedelta + from datetime import datetime from raven.config.loader import load_config @@ -1081,7 +1081,6 @@ def empty_totals() -> dict[str, Any]: selected_session = params.get("session_key") or None sessions: set[str] = set() - member_sessions: set[str] = {selected_session} if selected_session else set() models: dict[str, dict[str, Any]] = {} total = empty_totals() # The same resolution the writer uses (usage_tracker._default_telemetry_dir): @@ -1114,8 +1113,6 @@ def empty_totals() -> dict[str, Any]: sessions.add(root) if selected_session and root != selected_session: continue - if isinstance(row.get("session_key"), str): - member_sessions.add(row["session_key"]) name = str(row.get("model") or "?") acc = models.setdefault(name, {"model": name, **empty_totals()}) cost = reported_cost(row.get("cost_usd")) if row.get("schema_version") == 2 else None @@ -1138,7 +1135,6 @@ def empty_totals() -> dict[str, Any]: session_titles: dict[str, str] = {} tools: dict[str, int] = {} tool_total = 0 - telemetry_tool_ids: set[str] = set() try: for day in dates: p = tel_dir / f"usage-{day.isoformat()}.jsonl" @@ -1153,60 +1149,41 @@ def empty_totals() -> dict[str, Any]: if selected_session and root != selected_session: continue name = row.get("name") - if isinstance(row.get("tool_call_id"), str): - telemetry_tool_ids.add(row["tool_call_id"]) if isinstance(name, str): tools[name] = tools.get(name, 0) + 1 tool_total += 1 except Exception: continue + # Titles only. Tool calls were also counted from transcripts here, to + # cover conversations older than the day tool rows started being + # written, and a transcript counted as in-range when its file mtime + # was -- which gave the range every tool call the conversation had ever + # made while its model calls, dated per day, stayed outside it. A reply + # cannot carry two readings of one range: a page showing thousands of + # tool calls beside no model calls reads as broken, and is. sess_root = Path(load_config().workspace_path) / "sessions" - # Both ends, because the range is a window rather than a floor: the - # daily telemetry files this falls back for are read for the selected - # days only, so a transcript touched after `to` would add tool calls - # the other two tallies of the same reply do not have. + # A floor rather than a window: a transcript last written before the + # range cannot name a session the range saw, and which sessions it saw + # is what the telemetry above already answered. cutoff = datetime.combine(frm, datetime.min.time()).timestamp() - until = datetime.combine(to + timedelta(days=1), datetime.min.time()).timestamp() for p in sess_root.glob("*/*.jsonl"): try: - mtime = p.stat().st_mtime - if mtime < cutoff or mtime >= until: + if p.stat().st_mtime < cutoff: continue lines = p.read_text(encoding="utf-8").splitlines() except Exception: continue - metadata = None for line in lines: try: entry = json.loads(line) except ValueError: continue - if isinstance(entry, dict) and entry.get("_type") == "metadata": - metadata = entry - if metadata: - key = metadata.get("key") - title = (metadata.get("metadata") or {}).get("title") - if isinstance(key, str) and isinstance(title, str): - session_titles[key] = title - if selected_session and (not metadata or metadata.get("key") not in member_sessions): - continue - for line in lines: - try: - msg = json.loads(line) - except ValueError: + if not isinstance(entry, dict) or entry.get("_type") != "metadata": continue - if not isinstance(msg, dict): - continue - for tc in msg.get("tool_calls") or []: - if not isinstance(tc, dict): - continue - name = tc.get("name") or (tc.get("function") or {}).get("name") - call_id = tc.get("id") - if isinstance(call_id, str) and call_id in telemetry_tool_ids: - continue - if name: - tools[str(name)] = tools.get(str(name), 0) + 1 - tool_total += 1 + key = entry.get("key") + title = (entry.get("metadata") or {}).get("title") + if isinstance(key, str) and key in sessions and isinstance(title, str): + session_titles[key] = title except Exception: logger.exception("settings.usage: tool scan failed") diff --git a/raven/rpc/methods/session.py b/raven/rpc/methods/session.py index 77a380acb..1265fac0a 100644 --- a/raven/rpc/methods/session.py +++ b/raven/rpc/methods/session.py @@ -839,19 +839,23 @@ async def session_archive( agent_loop = _safe_invoke_factory(agent_loop_factory) config = load_config() mgr = manager_for(agent_loop, config) - session = mgr.get_or_create(session_key) # Restore writes an explicit False rather than dropping the key: the auto # archive pass in session.list skips any session that carries the key, so # a restored session stays out of its reach for good. - session.metadata["archived"] = archived - if mgr.exists(session_key): - try: - mgr.save(session) - except Exception: - logger.warning("session.archive: failed to persist archive state for {}", session_key) - return {"archived": archived, "session_key": session_key, "pending": True} - return {"archived": archived, "session_key": session_key, "pending": False} - return {"archived": archived, "session_key": session_key, "pending": True} + # + # One appended key rather than a saved session, the same way the auto + # archive pass writes it: ``save`` rewrites the whole metadata record from + # this process's copy of it, so a second client holding the conversation + # from before the archive dropped the flag on its next save of anything -- + # a title, a model -- and the conversation came back. + try: + persisted = mgr.append_metadata_patch(session_key, {"archived": archived}) + except Exception: + logger.warning("session.archive: failed to persist archive state for {}", session_key) + persisted = False + if not persisted: + mgr.get_or_create(session_key).metadata["archived"] = archived + return {"archived": archived, "session_key": session_key, "pending": not persisted} async def session_clear( diff --git a/raven/session/manager.py b/raven/session/manager.py index cda06ad0a..4d4bca31c 100644 --- a/raven/session/manager.py +++ b/raven/session/manager.py @@ -655,24 +655,32 @@ def _load(self, key: str) -> Session | None: logger.warning("Failed to load session {}: {}", key, e) return None - def append_metadata_patch(self, key: str, patch: dict[str, Any]) -> None: + def append_metadata_patch(self, key: str, patch: dict[str, Any]) -> bool: """Fold ``patch`` into a session's metadata by appending one record. The last metadata record wins on load, so appending is enough -- and the transcript is never read, which is what lets a housekeeping pass - touch hundreds of sessions without loading any of them. + touch hundreds of sessions without loading any of them. Merging into + the record on disk is also what keeps two clients from undoing each + other: whoever writes second keeps the other's keys, which a whole + ``save`` of one client's copy of the metadata cannot do. + + False when there was nothing to append to -- no transcript, or one with + no metadata record -- which a caller reporting whether a flag reached + the disk has to tell apart from a write that happened. """ path = self.session_path(key) if not path.is_file(): - return + return False last, _count, _last_ts, _first, _preview = self._scan_file(path) if last is None: - return + return False merged = {**(last.get("metadata") or {}), **patch} locked_append(path, [json.dumps({**last, "metadata": merged}, ensure_ascii=False)]) cached = self._cache.get(key) if cached is not None: cached.metadata.update(patch) + return True def save(self, session: Session) -> None: """Save a session to disk. diff --git a/tests/test_agents_code_launcher.py b/tests/test_agents_code_launcher.py index 0db81fb77..f50b50215 100644 --- a/tests/test_agents_code_launcher.py +++ b/tests/test_agents_code_launcher.py @@ -953,14 +953,16 @@ def test_the_products_tool_face_is_the_forks_config_intent_minus_the_ledger(grou # --- the permission gate: only the hosting with nobody to ask opens it ------------ -def test_the_acp_render_leaves_the_ask_tier_alone(grounded): - """Trunk's permission gate (permissions.mode, default ``ask``) prompts a - person before a write or a command, and refuses outright when the turn is - not interactive. The ACP hosting keeps that default on purpose: raven - dispatching a sub-agent answers those prompts itself - (``raven/acp_client/permissions.py`` approves every one), and a person in - an editor should still be asked -- opening the tier product-wide would - take their prompt away for good.""" +def test_the_acp_render_pins_the_ask_tier(grounded): + """The ACP hosting asks, whatever tier trunk defaults to. + + It used to inherit that default, which was the ask tier. The default has + since moved to smart, where a reviewer speaks for the ask tier and lets + most of it through -- a product decision about raven's own surfaces. This + is not one of them: the person is in an editor watching an agent work on + their checkout, and the prompt is how they see each write before it lands. + Raven dispatching a sub-agent is unaffected either way, since + ``raven/acp_client/permissions.py`` answers every prompt itself.""" from raven.config.loader import load_config config = load_config(_render(grounded)) diff --git a/tests/test_config_live.py b/tests/test_config_live.py index 00efb7483..f8152d2e4 100644 --- a/tests/test_config_live.py +++ b/tests/test_config_live.py @@ -422,7 +422,7 @@ def test_permissions_node_invalid_from_the_start_answers_defaults(tmp_path): path = tmp_path / "config.json" path.write_text('{"permissions": {"mode": "godmode"}}') fresh = permissions_config(LiveConfig(path)) - assert fresh.mode == "ask" + assert fresh.mode == "smart" assert fresh.tools == {} diff --git a/tests/test_rpc_config.py b/tests/test_rpc_config.py index 54a9ec012..22106e982 100644 --- a/tests/test_rpc_config.py +++ b/tests/test_rpc_config.py @@ -1085,9 +1085,9 @@ async def test_a_conversation_mode_stays_in_memory_and_off_the_default(fake_home own = await config_get({"keys": ["permissions.mode"], "session_id": "s-1"}) assert own["config"]["permissions.mode"] == "full" other = await config_get({"keys": ["permissions.mode"], "session_id": "s-2"}) - assert other["config"]["permissions.mode"] == "ask" + assert other["config"]["permissions.mode"] == "smart" default = await config_get({"keys": ["permissions.mode"]}) - assert default["config"]["permissions.mode"] == "ask" + assert default["config"]["permissions.mode"] == "smart" async def test_a_conversation_mode_moves_both_ways(fake_home: Path, own_mode) -> None: diff --git a/tests/test_rpc_session.py b/tests/test_rpc_session.py index 331892c78..4a820cea1 100644 --- a/tests/test_rpc_session.py +++ b/tests/test_rpc_session.py @@ -1831,6 +1831,93 @@ async def test_session_archive_persists_and_filters_the_list(tmp_path: Path, mon assert [row["id"] for row in (await session_list({}))["sessions"]] == [session_key] +async def test_archiving_keeps_a_key_another_writer_added(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Archiving speaks for the archived flag and for nothing else. + + It used to persist by saving the whole session, which rewrites the metadata + record from this manager's copy of it -- so every key written to the file + after that copy was loaded was dropped by an unrelated archive. A page and + a terminal over one home are two managers over one file, and that is how a + conversation came back after being archived: not because archiving failed, + but because somebody else's save spoke for a flag it had never seen. + """ + cfg = load_config() + cfg.agents.defaults.workspace = str(tmp_path) + monkeypatch.setattr(session_module, "load_config", lambda: cfg) + + session_key = "tui:20260610_100000_foreignkey" + mgr = SessionManager(tmp_path) + session = mgr.get_or_create(session_key) + session.add_message("user", "two writers hold this") + mgr.save(session) + monkeypatch.setattr("raven.session.resolve.build_manager", lambda cfg: mgr) + + # Somebody else writes a flag straight to the file; this manager's copy of + # the metadata knows nothing about it. + SessionManager(tmp_path).append_metadata_patch(session_key, {"pinned": True}) + assert mgr.get_or_create(session_key).metadata.get("pinned") is None + + result = await session_archive({"session_id": session_key, "archived": True}) + assert result == {"archived": True, "session_key": session_key, "pending": False} + + reloaded = SessionManager(tmp_path).peek(session_key) + assert reloaded is not None + assert reloaded.metadata.get("archived") is True + assert reloaded.metadata.get("pinned") is True + + +async def test_archiving_a_session_with_no_transcript_says_so(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Nothing reached the disk, and the reply says it rather than implying it. + + A conversation minted but never saved has no file to fold the flag into. + The call answers ``pending``, and a client that reads that as a success + takes the row off its list and finds it back on the next load. + """ + cfg = load_config() + cfg.agents.defaults.workspace = str(tmp_path) + monkeypatch.setattr(session_module, "load_config", lambda: cfg) + mgr = SessionManager(tmp_path) + monkeypatch.setattr("raven.session.resolve.build_manager", lambda cfg: mgr) + + session_key = "tui:20260610_100000_nofile" + result = await session_archive({"session_id": session_key, "archived": True}) + assert result == {"archived": True, "session_key": session_key, "pending": True} + assert not mgr.exists(session_key) + assert mgr.get_or_create(session_key).metadata.get("archived") is True + + +async def test_a_refused_write_is_reported_rather_than_swallowed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A filesystem that refuses the append answers pending, not success. + + The flag is still applied in memory so the session behaves as asked for as + long as this process lives, but the reply says it did not reach the disk -- + a client that takes the row off its list on a plain success would find it + back on the next load. + """ + cfg = load_config() + cfg.agents.defaults.workspace = str(tmp_path) + monkeypatch.setattr(session_module, "load_config", lambda: cfg) + + session_key = "tui:20260610_100000_refused" + mgr = SessionManager(tmp_path) + session = mgr.get_or_create(session_key) + session.add_message("user", "hello") + mgr.save(session) + monkeypatch.setattr("raven.session.resolve.build_manager", lambda cfg: mgr) + + def refuse(*_args: object, **_kwargs: object) -> bool: + raise OSError("read-only file system") + + monkeypatch.setattr(mgr, "append_metadata_patch", refuse) + + result = await session_archive({"session_id": session_key, "archived": True}) + assert result == {"archived": True, "session_key": session_key, "pending": True} + assert mgr.get_or_create(session_key).metadata["archived"] is True + assert SessionManager(tmp_path).peek(session_key).metadata.get("archived") is None + + async def test_session_archive_via_dispatcher(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: cfg = load_config() cfg.agents.defaults.workspace = str(tmp_path) diff --git a/tests/test_rpc_settings.py b/tests/test_rpc_settings.py index 64d284fd6..93517a694 100644 --- a/tests/test_rpc_settings.py +++ b/tests/test_rpc_settings.py @@ -1228,45 +1228,77 @@ async def test_usage_daily_buckets_cover_the_range_with_zero_days(telemetry): assert r["tools"]["counts"] == [{"name": "exec", "count": 1}] -def _transcript(home, name: str, days_ago: int, calls: list[str]) -> None: - """One session file whose tool calls the fallback scan may or may not count. +def _tool_row(name: str, call_id: str) -> dict: + return { + "_type": "tool_call", + "schema_version": 2, + "name": name, + "tool_call_id": call_id, + "session_key": "web:s1", + "root_session_key": "web:s1", + } + + +def _transcript(home, name: str, days_ago: int, calls: list[str], channel: str = "tui") -> None: + """One session file carrying tool calls, with its mtime set by ``days_ago``. - The scan reads a transcript when its mtime is inside the window, so the - mtime is what the case is about; the rows themselves are the same either - way. + Nothing in the reply may be counted off it -- the cases below are about a + transcript NOT reaching the tallies -- so the mtime is what they vary. """ import os from datetime import datetime, timedelta - d = home / "workspace" / "sessions" / "tui" + d = home / "workspace" / "sessions" / channel d.mkdir(parents=True, exist_ok=True) f = d / f"{name}.jsonl" - rows = [{"_type": "metadata", "key": f"tui:{name}", "metadata": {"title": name}}] + rows = [{"_type": "metadata", "key": f"{channel}:{name}", "metadata": {"title": name}}] rows.append({"role": "assistant", "tool_calls": [{"id": f"{name}-1", "name": c} for c in calls]}) f.write_text("\n".join(json.dumps(r) for r in rows) + "\n", encoding="utf-8") when = (datetime.now() - timedelta(days=days_ago)).timestamp() os.utime(f, (when, when)) -async def test_usage_tool_scan_is_bounded_at_both_ends(telemetry, tmp_path): - """A transcript touched after `to` is outside the window the reply reports. +async def test_usage_counts_a_tool_call_on_the_day_it_was_recorded(telemetry, tmp_path): + """One range read one way, tools included. - The LLM and telemetry-tool tallies read the selected days' files only, so - counting a transcript modified later made one reply disagree with itself: - the tool total covered a wider range than the dates beside it. + The tool tally used to be topped up by reading session transcripts, and a + transcript counted as in-range when its file mtime was -- which handed the + range every tool call that conversation had ever made, while its model + calls, dated per day, stayed outside it. A page showing thousands of tool + calls beside no model calls at all reads as broken, and is: one reply + cannot carry two readings of the same range. """ - _transcript(tmp_path, "inside", 4, ["exec"]) - _transcript(tmp_path, "after", 0, ["read_file", "read_file"]) - _transcript(tmp_path, "before", 40, ["grep"]) + telemetry(1, [_tool_row("exec", "in-1")]) + telemetry(40, [_tool_row("grep", "old-1")]) + # Touched today, so the old scan would have taken it, and full of tool + # calls that belong to no day this reply reports. + _transcript(tmp_path, "touched-today", 0, ["read_file", "read_file"]) - r = await rpc_console.settings_usage({"from": _iso(5), "to": _iso(3)}) + r = await rpc_console.settings_usage({"from": _iso(5), "to": _iso(0)}) assert r["tools"]["counts"] == [{"name": "exec", "count": 1}] assert r["tools"]["total"] == 1 - # And the same scan does count it once the range reaches that day. + # The day itself is what decides, not the file: widen the range and the + # older call joins; the transcript still does not. + r = await rpc_console.settings_usage({"from": _iso(41), "to": _iso(0)}) + assert sorted(c["name"] for c in r["tools"]["counts"]) == ["exec", "grep"] + assert r["tools"]["total"] == 2 + + +async def test_usage_names_the_sessions_the_range_saw(telemetry, tmp_path): + """Titles come from the transcripts of the sessions telemetry named. + + A transcript is read for its title and for nothing else, and only when the + range saw that session at all -- which the telemetry answers, so the file + system never gets a vote on what the range covers. + """ + telemetry(1, [_telemetry_row("a", 1.0)]) + _transcript(tmp_path, "s1", 0, [], channel="web") + _transcript(tmp_path, "elsewhere", 0, [], channel="web") + r = await rpc_console.settings_usage({"from": _iso(5), "to": _iso(0)}) - assert sorted(c["name"] for c in r["tools"]["counts"]) == ["exec", "read_file"] - assert r["tools"]["total"] == 3 + assert r["sessions"] == ["web:s1"] + assert r["session_titles"] == {"web:s1": "s1"} async def test_usage_from_is_clamped_and_reversed_range_refused(telemetry): diff --git a/ui-tui/src/__tests__/permCommand.test.ts b/ui-tui/src/__tests__/permCommand.test.ts index 743043475..3076ba3cd 100644 --- a/ui-tui/src/__tests__/permCommand.test.ts +++ b/ui-tui/src/__tests__/permCommand.test.ts @@ -63,12 +63,12 @@ describe('/perm', () => { expect(h.main[0]).toContain('default permission mode: ask') }) - it('falls back to ask only when the engine answers nothing', async () => { + it('falls back to the shipped default only when the engine answers nothing', async () => { const rpc = vi.fn(() => Promise.resolve({ config: {} })) const h = run('', rpc) await settle() - expect(h.main[0]).toContain('permission mode: ask') + expect(h.main[0]).toContain('permission mode: smart') }) it('writes a valid tier through config.set', async () => { diff --git a/ui-tui/src/app/slash/commands/core.ts b/ui-tui/src/app/slash/commands/core.ts index 65527ac19..7ebdec98f 100644 --- a/ui-tui/src/app/slash/commands/core.ts +++ b/ui-tui/src/app/slash/commands/core.ts @@ -163,7 +163,7 @@ export const coreCommands: SlashCommand[] = [ }) .then(r => ctx.transcript.sys( - `${isDefault ? 'default permission mode' : 'permission mode'}: ${r?.config?.['permissions.mode'] ?? 'ask'} (${usage})` + `${isDefault ? 'default permission mode' : 'permission mode'}: ${r?.config?.['permissions.mode'] ?? 'smart'} (${usage})` ) ) .catch(ctx.guardedErr) diff --git a/ui-web/scripts/__golden__/boot-live-noserver.txt b/ui-web/scripts/__golden__/boot-live-noserver.txt index a36dba1fa..e44a4f1a4 100644 --- a/ui-web/scripts/__golden__/boot-live-noserver.txt +++ b/ui-web/scripts/__golden__/boot-live-noserver.txt @@ -117,7 +117,6 @@ div.app[data-rail=on] svg.pico path path - path span#permName span#envChip.chip span.led diff --git a/ui-web/src/features/rail/wire.test.ts b/ui-web/src/features/rail/wire.test.ts new file mode 100644 index 000000000..1c1115796 --- /dev/null +++ b/ui-web/src/features/rail/wire.test.ts @@ -0,0 +1,71 @@ +// @vitest-environment happy-dom +/* What the rail does with the answer to an archive: the row leaves only when + * the call says the flag reached the disk. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { loadPart } from '../../../scripts/module-harness.mjs' + +interface Row { id: string; title: string } + +async function harness(answer: Record) { + const log: unknown[][] = [] + const rows: Row[] = [{ id: 'tui:1', title: 'a task' }] + await loadPart(() => import('./wire'), { + fakes: { + 'src/i18n/t': { t: (key: string, vars?: unknown) => (vars ? `${key} ${JSON.stringify(vars)}` : key) }, + 'src/state/toast': { show: (text: string) => { log.push(['toast', text]) } }, + 'src/features/rail/source': { + setArchived: async (id: string, archived: boolean) => { + log.push(['setArchived', id, archived]) + return answer + }, + deleteSession: async () => ({}), + }, + 'src/state/session/rows': { + rows: () => rows, + replace: () => {}, + open: () => {}, + }, + 'src/features/rail/store': { + draw: () => { log.push(['draw']) }, + removeSessionRow: (all: Row[], _cur: string | null, id: string) => { + log.push(['removeRow', id]) + return { kind: 'unchanged', rows: all.filter((r) => r.id !== id) } + }, + }, + 'src/lib/dom': { $: () => null }, + 'src/state/session/registry': { forget: () => {}, switchToDraft: async () => { log.push(['left']) } }, + 'src/lib/session': { current: () => null, setCurrent: () => {} }, + 'src/state/confirm': { ask: () => {} }, + 'src/state/sheetRack': { forget: () => {} }, + 'src/features/composer/mount': { dropDraft: () => {} }, + 'src/features/dag/mount': { forget: () => {} }, + 'src/features/settings/store': { redraw: () => {} }, + }, + }) + const wire = await import('./wire') + return { wire, log } +} + +afterEach(() => { vi.restoreAllMocks() }) + +describe('archiving from the rail', () => { + it('takes the row off once the flag is on disk', async () => { + const h = await harness({ archived: true, session_key: 'tui:1', pending: false }) + await h.wire.archive({ id: 'tui:1', title: 'a task' } as never) + expect(h.log).toContainEqual(['setArchived', 'tui:1', true]) + expect(h.log.some((c) => c[0] === 'toast' && String(c[1]).startsWith('gui.sess.archived'))).toBe(true) + }) + + it('says it failed when the call answers that nothing was persisted', async () => { + /* `pending` is the call reporting the flag is held in memory for a + conversation with no transcript yet. Read as a success it took the row + off the rail and left the reader to meet it again after a reload. */ + const h = await harness({ archived: true, session_key: 'tui:1', pending: true }) + await h.wire.archive({ id: 'tui:1', title: 'a task' } as never) + expect(h.log).toContainEqual(['setArchived', 'tui:1', true]) + expect(h.log.some((c) => c[0] === 'toast' && String(c[1]).startsWith('gui.sess.archive_failed'))).toBe(true) + expect(h.log.some((c) => c[0] === 'left')).toBe(false) + }) +}) diff --git a/ui-web/src/features/rail/wire.ts b/ui-web/src/features/rail/wire.ts index e1430112d..310f403a5 100644 --- a/ui-web/src/features/rail/wire.ts +++ b/ui-web/src/features/rail/wire.ts @@ -94,14 +94,20 @@ export async function archive(s: SessRow): Promise { try { const at = sessionRows().findIndex((row: SessRow) => row.id === s.id) const result = await setArchived(s.id, true) - if (!result.archived || result.session_key !== s.id) throw new Error(`session ${s.id} was not archived`) + /* `pending` is the call saying the flag is in memory only -- a conversation + with no transcript yet -- so nothing on disk changed and the next list + brings the row back. Reading it as a success took the row off the rail + and left the reader to find it again after a reload. */ + if (!result.archived || result.session_key !== s.id || result.pending) { + throw new Error(`session ${s.id} was not archived`) + } await leaveArchivedSession(s.id) toast(t('gui.sess.archived', { title: s.title }), { label: t('gui.undo'), fn: async () => { try { const restored = await setArchived(s.id, false) - if (restored.archived || restored.session_key !== s.id) { + if (restored.archived || restored.session_key !== s.id || restored.pending) { throw new Error(`session ${s.id} was not restored`) } if (!sessionRows().some((row: SessRow) => row.id === s.id)) { diff --git a/ui-web/src/features/settings/source.test.ts b/ui-web/src/features/settings/source.test.ts index 1244df9af..e6091ae8c 100644 --- a/ui-web/src/features/settings/source.test.ts +++ b/ui-web/src/features/settings/source.test.ts @@ -21,7 +21,7 @@ async function load(answers: Record = {}): Promise { fakes: { 'src/state/toast': { show: (text: string) => { toasts.push(text) } }, 'src/lib/openUrl': { open: (url: string) => { opened.push(url) } }, - 'src/features/rail/source': { loadSessions: async () => { railReloads.push(1) } }, + 'src/features/rail/source': { loadSessions: async () => { railReloads.push(1) }, SESS_CHANNELS: ['tui', 'cron'] }, 'src/state/banner': { draw: () => {} }, 'src/i18n/t': { t: (key: string, vars?: Record) => (vars ? `${key} ${JSON.stringify(vars)}` : key) }, }, @@ -83,10 +83,13 @@ describe('settings source', () => { expect(railReloads).toEqual([1]) }) - it('archived lists only the archived sessions', async () => { + it('archived lists the archived sessions of every channel the rail shows', async () => { + /* Fewer channels here than the rail lists means a row the rail archived is + invisible on this page and unreachable on that one: a scheduled run + could be archived and then neither restored nor deleted. */ const mod = await load({ 'session.list': { sessions: [{ id: 'a' }] } }) expect(await mod.settingsSource.archived()).toEqual([{ id: 'a' }]) - expect(seen).toEqual([['session.list', { archived: true }]]) + expect(seen).toEqual([['session.list', { archived: true, channels: ['tui', 'cron'] }]]) }) it('oauthLogin opens the verification page on this browser and returns the code', async () => { diff --git a/ui-web/src/features/settings/source.ts b/ui-web/src/features/settings/source.ts index b02a7e39f..a1c536e8e 100644 --- a/ui-web/src/features/settings/source.ts +++ b/ui-web/src/features/settings/source.ts @@ -30,7 +30,7 @@ import { setDefaultPair, showModel, } from '../model/source' -import { loadSessions } from '../rail/source' +import { loadSessions, SESS_CHANNELS } from '../rail/source' import type { ParamsOf, ResultOf } from '../../rpc/generated' import type { BannerSource } from '../../state/banner' @@ -258,7 +258,11 @@ export const settingsSource: SettingsSource = { pickModel: (model, provider) => run(persistModel(model, provider, 'default').then((r) => r === 'needs_restart')), model: () => defaultModel(), defaultProvider: () => defaultProvider(), - archived: () => gateway().call('session.list', { archived: true }).then((r) => r.sessions || []), + /* The same channels the rail lists, because this page is where a row the + rail archived has to show up: asking for fewer left an archived scheduled + run invisible here and unreachable there. */ + archived: () => gateway().call('session.list', { archived: true, channels: SESS_CHANNELS }) + .then((r) => r.sessions || []), /* The rail lists by its own read, so it is that read that puts the row back. */ restore: (id) => run(gateway().call('session.archive', { session_id: id, archived: false }) .then(() => loadSessions())), diff --git a/ui-web/src/features/settings/store.test.ts b/ui-web/src/features/settings/store.test.ts index 181f0bb82..35a6bf1b4 100644 --- a/ui-web/src/features/settings/store.test.ts +++ b/ui-web/src/features/settings/store.test.ts @@ -91,6 +91,22 @@ describe('settings store', () => { }) }) +describe('settings store, what a reopen drops', () => { + it('opening again clears what the pages fetched for themselves', async () => { + setSources({ settings: { load: async () => snapOf('m') } as unknown as SettingsSource }) + /* Both carry an answer from an earlier open. `refresh` reloads the + snapshot and cannot touch these two, so without the clear a dialog + opened once showed its first answer for the life of the page -- a + session archived from the rail in between never reached the archive + page, and the usage totals stayed at whatever they were on first open. */ + store.set({ usage: null, archived: [] }) + await store.open() + expect(store.get().usage).toBe(undefined) + expect(store.get().archived).toBe(null) + settingsDialog.close() + }) +}) + describe('settings store, the inventory push', () => { it('refreshSoon reloads once per burst while the dialog is open, and not at all while it is down', async () => { vi.useFakeTimers() diff --git a/ui-web/src/features/settings/store.ts b/ui-web/src/features/settings/store.ts index 6ea2113c8..8259d6448 100644 --- a/ui-web/src/features/settings/store.ts +++ b/ui-web/src/features/settings/store.ts @@ -211,6 +211,12 @@ export function redraw(): void { reopen shows the values it already has while the reload runs. */ export async function open(): Promise { redraw() + /* The two the pages fetch for themselves are not in the snapshot, so the + reload below cannot freshen them: a dialog opened once held its archive + list and its usage totals for the life of the page, and a session archived + from the rail in between never showed up. Dropping them here is what makes + each page ask again -- their own lazy loads already key off these two. */ + set({ usage: undefined, archived: null }) settingsDialog.open() await refresh() } diff --git a/ui-web/src/state/perm.test.ts b/ui-web/src/state/perm.test.ts index 897979c46..771f23f3c 100644 --- a/ui-web/src/state/perm.test.ts +++ b/ui-web/src/state/perm.test.ts @@ -51,13 +51,13 @@ const pop = (): HTMLElement => document.getElementById('permPop')! const rows = (): HTMLElement[] => [...document.querySelectorAll('#permList .prow')] describe('the permission chip', () => { - it('defaults to ask, the product default the gate ships with', async () => { + it('defaults to smart, the product default the gate ships with', async () => { const perm = await load() - expect(perm.current()).toBe('ask') + expect(perm.current()).toBe('smart') perm.draw() - expect(document.getElementById('permName')!.textContent).toBe('gui.perm.ask') + expect(document.getElementById('permName')!.textContent).toBe('gui.perm.smart') expect(chip().classList.contains('risk')).toBe(false) - expect(chip().getAttribute('aria-label')).toBe('gui.perm.title: gui.perm.ask') + expect(chip().getAttribute('aria-label')).toBe('gui.perm.title: gui.perm.smart') }) it('marks the risky tier on the chip itself', async () => { @@ -78,16 +78,16 @@ describe('the permission chip', () => { it('ignores a stored tier no build offers any more', async () => { const perm = await load('godmode') - expect(perm.current()).toBe('ask') + expect(perm.current()).toBe('smart') }) it('takes the mode the live layer loaded from config', async () => { const perm = await load() - perm.setFromConfig('smart') - expect(perm.current()).toBe('smart') - expect(localStorage.getItem('raven.perm')).toBe('smart') + perm.setFromConfig('ask') + expect(perm.current()).toBe('ask') + expect(localStorage.getItem('raven.perm')).toBe('ask') perm.setFromConfig('godmode') - expect(perm.current()).toBe('smart') + expect(perm.current()).toBe('ask') }) it('puts the tier icon in the chip slot', async () => { @@ -111,7 +111,7 @@ describe('the permission popover', () => { 'gui.perm.full', ]) expect(rows().map((r) => r.getAttribute('role'))).toEqual(['radio', 'radio', 'radio']) - expect(rows().map((r) => r.getAttribute('aria-checked'))).toEqual(['true', 'false', 'false']) + expect(rows().map((r) => r.getAttribute('aria-checked'))).toEqual(['false', 'true', 'false']) /* Only the risky tier wears the class, and only the chosen one has a tick. */ expect(rows().filter((r) => r.classList.contains('risk')).length).toBe(1) expect(pop().querySelectorAll('svg.tick').length).toBe(1) @@ -237,7 +237,7 @@ describe('persisting a pick', () => { rows()[2]!.click() await Promise.resolve() await Promise.resolve() - expect(perm.current()).toBe('ask') + expect(perm.current()).toBe('smart') expect(localStorage.getItem('raven.perm')).not.toBe('full') }) }) diff --git a/ui-web/src/state/perm.ts b/ui-web/src/state/perm.ts index 1523bf84b..1328e8146 100644 --- a/ui-web/src/state/perm.ts +++ b/ui-web/src/state/perm.ts @@ -39,6 +39,13 @@ export const TIERS: readonly Tier[] = [ const KEY = 'raven.perm' +/* What the chip shows before the config has loaded, and the only tier the page + can name on its own. It is the engine's default (raven/config/schema.py, + PermissionsConfig.mode) written twice: the page paints before the config + arrives, and painting a tier the engine is not in reads as a mode flipping + under the reader. */ +const DEFAULT_TIER = 'smart' + /* Shields, one per tier, differing only in what is inside them: a question, a check, an exclamation. Same outline so the three read as one control's three states rather than three unrelated icons. */ @@ -134,7 +141,7 @@ function read(): string { } catch { stored = '' } - return TIERS.some((p) => p.id === stored) ? stored : 'ask' + return TIERS.some((p) => p.id === stored) ? stored : DEFAULT_TIER } function remember(value: string): void {