Skip to content

Re-sync the Notebook panel after notebook.md writes that bypass the watcher#363

Open
dannon wants to merge 1 commit into
galaxyproject:mainfrom
dannon:fix/253-notebook-panel-autorefresh
Open

Re-sync the Notebook panel after notebook.md writes that bypass the watcher#363
dannon wants to merge 1 commit into
galaxyproject:mainfrom
dannon:fix/253-notebook-panel-autorefresh

Conversation

@dannon

@dannon dannon commented Jun 26, 2026

Copy link
Copy Markdown
Member

Closes #253

The Notebook panel only refreshed when the chokidar watcher caught a change to notebook.md, but the watcher tracks a single file path. An atomic temp-and-rename save (sed -i, an editor) swaps the inode out from under it, and a plain truncate can race its awaitWriteFinish window -- so when the agent wrote the notebook through bash instead of /notebook, the panel went stale. The tool_execution_end hook also only re-emitted for galaxy_ tools.

What this does

tool_execution_end now calls a new reemitNotebookIfChanged() (in state.ts) after every tool. It re-reads notebook.md and emits through the existing notifyNotebookChange path only when the content actually changed:

  • a dev:ino:size:mtimeMs:ctimeMs stat fingerprint skips the read when the file is untouched, so it isn't I/O churn on every tool end, and
  • a content comparison suppresses no-op emits.

Including ino/ctime in the fingerprint (not just mtime+size) is deliberate: an inode-replacing or same-size rewrite with a preserved/coarse mtime is exactly the watcher-evading write this is meant to catch.

Two small hardening changes fell out of review of the new emit path:

  • notifyNotebookChange now isolates per-listener exceptions (logged, not rethrown), so one misbehaving subscriber can't break the tool_execution_end hook or starve other listeners.
  • setNotebookPath resets the emit baselines, so switching to a new notebook that happens to share the old one's content/fingerprint still re-syncs the panel instead of being suppressed.

Scope

This is the contained UI re-sync fix only -- the watcher itself is unchanged (parent-dir watching / a watcher rewrite is a separate, larger change). Two deliberately out-of-scope notes:

  • Duplicate emit in the race window: if reemit fires before the watcher's 500ms awaitWriteFinish settles, the watcher later emits the same content again. The only onNotebookChange listener (the UI bridge) dedups on content, so there's no visible double-render.
  • Auto-commit: a write the watcher truly misses refreshes the panel but isn't auto-committed (the watcher owns commits). That's a durability concern for the watcher work, not this UI fix, and it's not a regression -- previously such a write neither refreshed nor committed.

Tests

New tests/notebook-reemit.test.ts (7 cases): changed -> emit once, unchanged -> no emit, no re-emit on a second call, no-op without an active notebook, an inode-replacing write with pinned size+mtime still emits (fails on a mtime+size-only guard), a throwing listener neither breaks the caller nor starves other listeners, and a path switch to a same-content notebook still re-emits. The cases run with the real watcher stopped so they're deterministic. Full root suite green (1257 passing); root and Orbit typechecks clean.

…atcher

The panel only refreshed when the chokidar watcher caught a change, but the
watcher tracks a single file path -- an atomic temp-and-rename save (sed -i, an
editor) swaps the inode out from under it, and a plain truncate can race its
awaitWriteFinish window -- so a notebook written through bash went stale until
the next /notebook. The tool_execution_end hook also only re-emitted for
galaxy_ tools. It now calls reemitNotebookIfChanged() after every tool, which
re-reads notebook.md and emits through the existing notifyNotebookChange path
only when the content actually changed. A dev/ino/size/mtime/ctime stat
fingerprint skips untouched files (so it isn't churn on every tool end) and
catches inode-replacing or same-size rewrites that a mtime+size check would
miss; a content compare suppresses no-op emits. notifyNotebookChange now
isolates per-listener exceptions so a bad subscriber can't break the hook, and
switching notebook paths resets the emit baselines.
@dannon dannon force-pushed the fix/253-notebook-panel-autorefresh branch from dc0b7ff to 9a1382d Compare June 26, 2026 13:32
@dannon dannon marked this pull request as ready for review June 26, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notebook panel doesn't auto-refresh after the agent writes notebook.md (e.g. via bash); only /notebook refreshes it

1 participant