Skip to content

Commit 4abe67d

Browse files
ghackettclaude
ghackett
and
claude
committed
Split: the editor's files over the API (PR-2.3)
The editor opens no file of the project's and writes none: fs.read, fs.write (expect_mtime, refused stale with the file untouched) and fs.watch / fs.unwatch pushing file-changed are served by service/files.py's Files, confined by files.allowed, every read, write and stat on a worker thread behind a Deferred; remotefiles.py is the client's end and editor.py's open_file, _save, save_all, _reload_from_disk, _check_external and _retarget_open run over it, GtkSource.File, FileLoader and FileSaver gone. A text over the frame cap crosses chunked either way (split_request / join_request beside split_reply). tests/test_files.py, test_remotefiles.py, the protocol pins, two chunking cases in test_api_server.py; check_editor_save.py, and check_editor_narrow.py against a scratch service. The pathless allowlist loses the editor's three sites. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_011sbC3wuGjnM1AysiE7RjJ8
1 parent 2192055 commit 4abe67d

22 files changed

Lines changed: 2212 additions & 310 deletions

‎.agents/collins-editor-panel/SKILL.md‎

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,22 +54,70 @@ an install hint (`editor.py` import guard) — `prview` imports GtkSource
5454

5555
## Behaviours
5656

57-
**Opening** (`open_file(path, restore_cursor)`): guarded by
58-
`editorfiles.load_guard` (size, binary, image) and `is_inside(root)`;
59-
language from `guess_language_id` (extension, then the first line's shebang;
60-
its sibling `fence_language_id` maps a markdown fence's info word — `python`
61-
→ `python3`, `bash` → `sh` — for the PR page's code blocks in `mdwidgets`);
62-
style scheme from `editor.style_scheme(setting, dark)` (a bare
63-
`GtkSource.Buffer` defaults to the light `classic` scheme — never leave it
64-
unset). Cursor placement on a fresh buffer must re-issue `scroll_to_mark` from
65-
a `PRIORITY_LOW` idle (`_apply_cursor`): line heights are estimates until
57+
**Opening** (`open_file(path, restore_cursor)`): guarded by `is_inside(root)`
58+
and `editorfiles.is_image_path`; everything else about the file is the
59+
service's (below). Language from `guess_language_id` (extension, then the
60+
first line's shebang — `editorfiles.first_line` of the text the service
61+
read; its sibling `fence_language_id` maps a markdown fence's info word —
62+
`python` → `python3`, `bash` → `sh` — for the PR page's code blocks in
63+
`mdwidgets`), switched off above 512 KiB (`should_highlight(size)`); style
64+
scheme from `editor.style_scheme(setting, dark)` (a bare `GtkSource.Buffer`
65+
defaults to the light `classic` scheme — never leave it unset). Cursor
66+
placement on a fresh buffer must re-issue `scroll_to_mark` from a
67+
`PRIORITY_LOW` idle (`_apply_cursor`): line heights are estimates until
6668
validation idles run, so an immediate scroll lands ~line 44 for a target of
6769
602. `open_in_editor` (the MCP tool, the terminal's Ctrl+click on a path,
6870
"Add to chat" in reverse) all land in `MainWindow.open_in_tab_editor`.
6971

70-
**External changes**: each open file has a `Gio.FileMonitor`; a clean buffer
71-
reloads silently, a dirty one is told. The agent rewrites these files
72-
constantly, so this path is exercised more than manual saves are.
72+
**The files are the service's** (split-service spec §3.23, PR-2.3). The
73+
pane opens no file and writes none: `GtkSource.File`, `FileLoader` and
74+
`FileSaver` are gone, and every path is a path on the service's machine.
75+
`collins/remotefiles.py` (GTK-free) is the client's end, `collins/service/
76+
files.py` the service's:
77+
78+
- `_start_load` asks `fs.read` from a worker thread (`_off_main`: a daemon
79+
thread, the answer landed at `GLib.PRIORITY_DEFAULT`) and `_on_loaded`
80+
fills the buffer outside the undo history (`_fill`), keeping the reply's
81+
`mtime` and `encoding` on the `_OpenFile`. The guards moved with the
82+
read: the service refuses a path that is not a regular file, one over
83+
5 MiB (`protocol.FILE_TEXT_MAX`) and one outside every root it knows
84+
(`files.allowed`: a live session's cwd, a store session's cwd or project
85+
root, anything for a `local` client), and flags `binary` (a NUL in the
86+
first 8 KiB) and `latin-1` (bytes that are not UTF-8, written back the
87+
same way). A refusal's words come translated through
88+
`remotefiles.refusal_words` into the banner; a fresh open closes its tab,
89+
a reload keeps it. `load_id` still makes a superseded read (a rename's)
90+
a no-op.
91+
- `_do_save` sends `fs.write` with the buffer's text and `expect_mtime`:
92+
Ctrl+S (`_save`) expects the mtime of the last read or write, and a file
93+
that moved underneath is refused `stale` by the service with **nothing
94+
written** — `_on_saved` raises the "changed on disk" dialog, whose
95+
Overwrite saves again with `expect_mtime` null. `save_all` and the close
96+
flows' Save pass null from the start (the user's explicit consent, as
97+
before). The service writes by a temp file in the directory and one
98+
`os.replace`, keeps the mode and follows a symlink to the file.
99+
- `_watch_external_changes` installs `fs.watch kind: file` under a handle
100+
the client mints (`remotefiles.Watcher`; `_teardown_page` unwatches);
101+
the service's `Gio.FileMonitor` debounces 300 ms, stats on a thread and
102+
pushes `file-changed {handle, path, mtime, size, gone}` once per burst
103+
whose stat moved. `_check_external` judges it against `opened.mtime`: a
104+
clean buffer reloads silently (cursor kept), a dirty one is told
105+
(Reload), `gone` marks the buffer dirty and says so. An event that
106+
arrives while a save or load is in flight waits (`pending_change`) for
107+
the reply's mtime, so the editor's own write never reads as a change.
108+
The link is installed behind the `files` capability of the service's
109+
hello (`App._install_file_transport`, decided on every connect like
110+
git's); against a service without it nothing is sent and every open
111+
lands in the banner. A reconnect (`remotefiles.reset()` in
112+
`App._on_connected`) re-sends every live watch. The agent rewrites these files constantly, so this path is
113+
exercised more than manual saves are.
114+
- A text over the 1 MiB frame cap crosses as `TAG_BLOB` chunks either way
115+
(`protocol.split_request` / `split_reply`, the `text_chunked` /
116+
`text_bytes` fields), so a 5 MiB file saves.
117+
- `check_editor_save.py` drives all of it against a scratch service through
118+
`e2e_service.harness_link()` plus `remotefiles.install(link)`; a widget
119+
check that opens files needs the same two lines, or every open lands in
120+
the banner with "Not connected to the service".
73121

74122
**Following the session** (`request_root` / `offer_root`): the tab's cwd tick
75123
calls `_maybe_follow_editor`; `editorfiles.follow_scope(root, cwd)` and
@@ -137,4 +185,6 @@ close state joins all three.
137185
the missing-typelib exit.
138186

139187
Related: `collins-terminal-tab`, `collins-panel-dock`,
140-
`collins-gtk-sharp-edges`, `collins-testing` (`check_editor_narrow.py`).
188+
`collins-gtk-sharp-edges`, `collins-testing` (`check_editor_narrow.py`,
189+
`check_editor_save.py`), `collins-session-mcp-tools` (the API's message
190+
table in `api/protocol.py`: the `fs.*` types).

‎.agents/collins-session-mcp-tools/SKILL.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,21 @@ import it. It holds:
108108
`gone`, `refused`, `failed`); a receiver accepts any code-shaped string.
109109
`msgid` is an English source string with `{name}` placeholders, for the
110110
client's `i18n._()` and `format_map`.
111+
- Files over the API (§3.23, PR-2.3; the `files` cap): `fs.read {path,
112+
max}` answers a file's `text`, `encoding` (`utf-8` or `latin-1`),
113+
`mtime` (microseconds), `size` and `binary`; `fs.write {path, text,
114+
expect_mtime, encoding}` is refused `stale` with nothing written when
115+
the file's mtime moved from `expect_mtime` (null writes regardless);
116+
`fs.watch {path, kind, handle}` / `fs.unwatch {handle}` are per-client
117+
`Gio.FileMonitor`s on the service pushing `file-changed {handle, path,
118+
mtime, size, gone}`, debounced 300 ms. The handlers are
119+
`service/files.py`'s `Files`, the client's `remotefiles.py`. A message
120+
over the frame cap is chunked **either way**: `split_message` /
121+
`join_message` on the first of `CHUNKED_FIELDS` (`stdout`, `text`) a
122+
message holds, as `TAG_BLOB` frames under the request id ahead of a slim
123+
message saying `<field>_chunked` / `<field>_bytes`; the server joins a
124+
chunked request (`join_request`) before validating it, the client's link
125+
a chunked reply (`join_reply`).
111126
- Framing: `encode` (refuses over `MAX_FRAME`, 1 MiB, and NaN / lone
112127
surrogates) and `decode` (refuses over `MAX_INCOMING`, 16 MiB, NaN,
113128
non-objects, nesting Python can't parse). The binary header is

‎collins/api/client.py‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,31 @@ def do_send():
448448

449449
self._post(do_send)
450450

451+
def _send_request(self, channel: _Channel, message: dict) -> None:
452+
"""A request on *channel*: one text frame, or, for one the frame cap
453+
can't hold (`protocol.split_request`: a `fs.write`'s text), its
454+
TAG_BLOB frames ahead of the slim request on the same connection.
455+
Raises RequestRefused ``invalid`` for a request too large even so."""
456+
try:
457+
frames, slim = protocol.split_request(message)
458+
except ValueError as exc:
459+
raise RequestRefused(protocol.ERROR_INVALID, f"{message.get('t')}: {exc}", {}) from None
460+
if frames:
461+
encoded = [GLib.Bytes.new(frame) for frame in frames]
462+
463+
def do_send_frames():
464+
ws = channel.ws
465+
if ws is None or ws.get_state() != Soup.WebsocketState.OPEN:
466+
return
467+
try:
468+
for frame in encoded:
469+
ws.send_message(Soup.WebsocketDataType.BINARY, frame)
470+
except GLib.Error as exc:
471+
log.error("api client: send failed: %s", exc)
472+
473+
self._post(do_send_frames)
474+
self._send_text(channel, slim)
475+
451476
def _blocking(self, channel: _Channel, message: dict, timeout: float) -> dict:
452477
"""Send on *channel* and wait for the reply (any thread but the I/O
453478
one)."""
@@ -463,7 +488,12 @@ def _blocking(self, channel: _Channel, message: dict, timeout: float) -> dict:
463488
if channel.ws is None:
464489
raise RequestRefused(protocol.ERROR_GONE, "Not connected to the service", {})
465490
channel.pending[message["id"]] = pending
466-
self._send_text(channel, message)
491+
try:
492+
self._send_request(channel, message)
493+
except RequestRefused:
494+
with self._lock:
495+
channel.pending.pop(message["id"], None)
496+
raise
467497
if not pending.event.wait(timeout):
468498
with self._lock:
469499
channel.pending.pop(message["id"], None)
@@ -557,7 +587,15 @@ def send(self, message: dict, on_reply=None, on_refused=None) -> None:
557587
log.warning("api client: %s refused: not connected", checked.type)
558588
return
559589
self._primary.pending[message["id"]] = pending
560-
self._send_text(self._primary, message)
590+
try:
591+
self._send_request(self._primary, message)
592+
except RequestRefused as refusal:
593+
with self._lock:
594+
self._primary.pending.pop(message["id"], None)
595+
if on_refused is not None:
596+
on_refused(refusal)
597+
else:
598+
log.warning("api client: %s refused: %s", checked.type, refusal.msgid)
561599

562600
def send_event(self, message: dict) -> None:
563601
"""A client event on the primary (`resize`, `focus`, `theme`,
@@ -625,7 +663,7 @@ def _on_chunk(self, channel: _Channel, payload: bytes) -> None:
625663
if not wanted:
626664
return # no request of ours: a late chunk, or not for this link
627665
buffer = channel.blobs.setdefault(header.stream, bytearray())
628-
if header.offset != len(buffer) or len(buffer) + len(data) > protocol.GIT_OUTPUT_MAX:
666+
if header.offset != len(buffer) or len(buffer) + len(data) > protocol.CHUNKED_MAX:
629667
channel.blobs.pop(header.stream, None) # out of order or over the bound: the reply is refused
630668
return
631669
buffer += data

0 commit comments

Comments
 (0)