Skip to content

Edit a score that runs on another machine - #2180

Open
jcelerier wants to merge 51 commits into
stack/plugin-robustnessfrom
remote-control
Open

Edit a score that runs on another machine#2180
jcelerier wants to merge 51 commits into
stack/plugin-robustnessfrom
remote-control

Conversation

@jcelerier

@jcelerier jcelerier commented Aug 8, 2026

Copy link
Copy Markdown
Member

Stacked on #2179 — review that one first; this targets its branch, so the
diff here is only the session work.

Paired with ossia/iscore-addon-network#9. The two repositories
change together, so ci/common.deps.sh pins the add-on to its matching
branch; that pin comes out when both are merged.

A terminal is a document that shows a score running elsewhere: it holds no
devices, runs no execution, and opens no files of its own.

  • score::Environment — a document says where its files are, and code asks
    rather than assuming a path is openable here
  • score::Uri<PROJECT>:, <LIBRARY>:, <CACHE>:, so a stored path
    means the same thing on both machines
  • dropped and picked files travel to the machine that will open them,
    content-addressed, rather than naming a path only the sender has
  • device presets, the library and shader previews are read through the
    environment
  • a device whose protocol only the host has can be added from a terminal
  • the host reports what its devices see, what its tree contains, and what it
    prints; a script typed in a terminal's console runs where the score runs
  • wss alongside ws, so a browser served over https can reach a host that
    is not its own machine

Draft while the last review round is worked through. Known and recorded:
the device-tree broadcast re-serializes a whole device per changed node, and
media already referenced by a document does not yet reach a terminal.

jcelerier and others added 20 commits August 17, 2026 21:05
A score names hardware -- sound cards, MIDI ports, cameras, OSC sockets,
render windows -- and opening one has always meant claiming all of it,
because the machine holding the document was the machine running the
show. That stops being true when the score is edited from somewhere
else: a laptop driving a headless box must not open that box's MIDI
ports on itself, and a browser has none of it to offer.

score::DocumentRole answers "may this document drive this machine". It
is fixed at construction rather than set afterwards, because devices are
instantiated while the device plug-in deserializes -- anything decided
after loadDocument returns is decided too late. Threaded through the one
Document constructor and the one DocumentBuilder overload that take
serialized data, both defaulted, so no existing call site changes.

Distinct from score::Environment on purpose: a peer in a multiplayer
session reads its score from another machine and still plays it on its
own hardware, so files-elsewhere and hardware-elsewhere are independent
questions. Conflating them would break multiplayer.

Terminal changes four things:
 - loadDeviceFromNode / createDeviceFromNode do not instantiate. The
   node stays in the tree with nothing behind it, which is the shape a
   missing protocol already produces, so the rest of score copes. Gfx
   output windows fall out of this: a Window device is just a device.
 - play_interval refuses. Transport belongs to the host.
 - the local tree is not created: it is a control surface for a score
   that executes here, and it would bind the same default ports as the
   machine actually running it.

Independently, ensure_audio_engine called qFatal without a main window,
so an unattended --no-gui host was killed by a play request it could
have declined. It is now has_audio_engine(), and callers decline.

Also fix the JSON save path, which asserted that every device node has a
live implementation: saving a document naming a protocol this build does
not have aborted. Write the full node in that case -- its addresses
exist only in the document, and dropping them empties the device for
every machine that does have the protocol.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A protocol the build genuinely has but that builds no device, so that
"score did not ask for a device" is a decision under test rather than an
absence. Used by the terminal-role tests on both sides of the addon
boundary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The guard was after m_localDeviceWrapper.init(), which is the call that
actually opens the OSCQuery server -- so a terminal still bound score's
own control ports, verified by two real instances on one machine where
the second reported "could not expose score on port 6666". Skip the
whole setup instead: DeviceList::localDevice() then stays null, which
Execution::DocumentPlugin already null-checks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
play_interval declined, but the transport state machine sets the buttons
to "playing" on its way there: trigger_play calls TransportActions
before play_interval ever runs. So pressing Play on a terminal left a
Play that looked pressed, a Stop with nothing to stop, and no way back
-- reported as "hitting play stays stuck".

Refuse at the request_* entry points instead, so the state machine is
never entered.

Not covered by a test: the button state comes from TransportActions,
which early-returns without the widgets a real window creates, so
headless the assertion passes with the guard removed. Verified in the
GUI instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
…phic kind

readFromAbstract has always written each polymorphic object into its own
blob, with a comment saying why: "so that we have a chance to save it
as-is and reload it later if the plug-in is not found on the system".
Processes, ports, document plug-ins and curve segments all get that.
DeviceSettings did not -- it wrote the protocol's settings inline into
the parent stream -- and every consequence followed from that one
exception:

 - a reader without the factory could not skip the payload, so it landed
   mid-payload and blamed the file for being corrupt
 - the workaround was to peek for the delimiter and guess whether a
   payload was there at all, which misreads any protocol whose settings
   happen to begin with those four bytes
 - throwMissingProtocol existed to apologise for the whole situation
 - and commands, which are binary, could not carry a device for a
   protocol the receiving peer lacks: adding one to a session diverged
   every client that did not have it

Write it in a blob of its own, tagged with its format via
OpaquePayload. Tagged unconditionally, including when the factory was
present: a peer that has the factory must not be handed a payload
written by one that did not and mistake it for its own. In exchange a
build that *does* have the protocol can now read settings that arrived
as JSON, so a document that has been through another machine stays
readable rather than merely preserved.

throwMissingProtocol and the delimiter guess are both deleted.

This changes the .scorebin layout for devices. Binary documents have
never had a version check or a migration path -- DocumentManager's
updateJson is JSON-only -- so they are already same-version-only, and
this is the ordinary kind of breakage rather than a new one.

Also stop loadCommandStack failing a load over history it cannot read:
instantiateUndoCommand throws on an unknown command key, and for a
device it threw from inside deserialization, so a host's undo stack
could refuse a join outright. Truncate the history there instead --
losing undo past that point is much smaller than not opening at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
loadMissing rescues deserialization, and nothing rescued creation:
AddOnlyProcessToInterval::redo asserted on the factory, which in release
throws -- out of a socket callback, so the peer marked itself diverged
and stopped applying anything ever again. That is the reported "if the
client doesn't know the process it disconnects and never executes
commands again".

Two commands create processes and they need different answers:

 - AddOnlyProcessToInterval carries only what a factory would have been
   *given*, so there is nothing to keep. makeMissing returns a stand-in
   that reports itself incomplete: its emptiness is not authoritative,
   and writing it out as empty would tell a machine that has the plug-in
   that the process is empty when it is only unknown here.
 - LoadOnlyLayerInInterval carries the process serialized, so nothing is
   lost: loadMissing keeps the data verbatim and rebuilds the ports, and
   the process keeps its controls and its cables.

The second is the interesting one -- it already had "TODO handle missing
process" -- because it shows what a complete answer looks like: the
state has to come from somewhere, and where a command carries it there
is no problem at all. What the first needs is that state fetched from
the peer that could make it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A stand-in made for a creation command is empty, and its emptiness says
nothing about the object -- only about this build. setState fills it
from the object's own serialization, through the same path that loads
one from a document: ports rebuilt, controls back, placeholder gone.

awaitingState() is how the code that can ask finds them. Walking the
document after every replicated command is not affordable at a
control's update rate, and only the creating constructor produces one,
so it records itself there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The panel is one and documents are many, and what is available is a
property of the document: a score that runs on another machine can only
use that machine's processes. So the list is rebuilt from this build's
factories whenever a document that runs here becomes visible, and left
alone for one that does not, since whatever mirrored the other machine
owns it then. Without this, opening a local document beside a terminal
left the library showing the wrong machine's world.

The GUI test that guards it found another instance of the
device-with-no-implementation family: ListeningManager::deviceFromNode
went through DeviceList::device(), and it is reached from the explorer
*view* when a device row is inserted -- so only a test with real widgets
could hit it, and the earlier headless ones could not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
"Add a device" has always meant this machine's protocols and this
machine's hardware, because the score ran here. For a score that runs
elsewhere it has to mean that machine's: its MIDI ports, its cameras,
its protocols. Ours are unreachable from there, so offering them offers
something that can never happen.

Device::DeviceCatalog is where the dialog asks, set on the document like
score::Environment is and null for an ordinary one, which keeps the
existing behaviour exactly. Asynchronous for the same reason Environment
is -- the answer may come over a socket -- which fits a dialog that
already fills its list from enumerator signals.

One limit is real and stated rather than worked around: a protocol this
build does not have has no settings widget, because the widget is C++ in
a plug-in we do not have. Such a protocol can still be used through what
the other machine enumerates, since those come with the settings the
protocol itself wrote. The form says so instead of appearing empty.

Also null-guard the two makeSettingsWidget calls and getEnumerators:
they went through ProtocolFactoryList::get() unchecked, which was
unreachable while the list was the only source of keys and is reachable
the moment it is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
QFileSystemModel cannot do this, and not by oversight: it is built on
paths this process can stat, and a listing that crosses a socket is
neither synchronous nor local. That is the same reason score::Environment
is asynchronous, so RemoteFileSystemModel is the model that fits it --
lazy, listing a folder when it is first opened and keeping the answer.

The user-library panel now switches between the two by document: local
documents keep the QFileSystemModel browser exactly as before, and one
whose score runs elsewhere gets the other machine's library.

Dragging deliberately does not produce text/uri-list. That means files
this process can open, and every drop handler treats it that way; these
are not on this machine, so a file:// URL would name something that is
not there and each handler would try to open it. They travel as
score::Uri under a type of their own, which handlers that do not know it
ignore -- the correct outcome until one does. Making a drop actually
land is the next step and needs the drop handlers to accept a Uri.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A peer that does not run the score has no DeviceInterface behind any of
its device nodes, so asking one reported everything as disconnected.
That is not "unknown", it is wrong: the devices are connected, just not
here.

The explorer now prefers what the machine running the score reports, and
falls back to the local device when there is nothing -- which is every
ordinary document, unchanged.

Also fix the enumerated-device list in the add-device dialog. With a
catalog the local enumerator vector stays empty, so the code fell
through to the branch that hides the device column because there is
nothing in it yet -- and what would have re-shown it is exactly the
answer being waited for. Shown up front instead, and left shown if
nothing arrives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
getCapas went through DeviceList::device(), which is a SCORE_ASSERT --
and SCORE_ASSERT *throws* in release. It runs from updateActions and
from the drag handler, so selecting or dragging a device whose protocol
this build does not have surfaced as "Internal error: Assertion
failure", from SafeQApplication's catch-all. Default capabilities
instead: not being able to ask is not the same as the device being
incapable.

Three more of the same in the context menu, the JS device iteration and
the recording manager. That is now every DeviceList::device() call in
the tree; the remaining ones are QDataStream::device().

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A command carries what a factory would be *given*, not what the object
would *write*, and what it is given can describe a world this machine is
not in. A library entry for an ISF shader carries the path of the file
it was scanned from: the factory exists in the wasm build, succeeds, and
produces a process with no controls, because the file is on the host. A
missing factory was only the loudest case of the same thing.

So the registry is no longer about stand-ins. On a document whose score
runs elsewhere, anything a replicated creation command builds is
registered as awaiting its real state -- Process::awaitingRemoteState(),
which OpaqueProcessModel's creation constructor also feeds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
rescan() resets a RecursiveWatch that is a function-local static shared
by every ProcessesItemModel, and restarts an asynchronous scan on the
task pool. Rescanning whenever a document became visible therefore raced
the scan the model's own constructor had just started: the second
rescan reset the watch while the first scan was still walking it, and
the process died on a worker thread inside the filter callback.

Intermittent by nature -- it only crashed when the startup scan was
still running -- which is why it passed here and killed the host on a
machine with a larger library. That host dying is what looked like "the
terminal sees no devices and no library": there was nothing left to
answer it.

Only rescan when the list currently describes another machine, which is
the only case that needs restoring: a document that runs here replacing
one that did not.

Left alone, and worth knowing: rescan() resetting a shared watch mid-scan
is unsafe in general, and the settings' rescanLibrary signal can still
trigger it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
execution_time() returns zero without a clock, and a terminal never has
one -- the score runs elsewhere. So the cursor moved and the time stayed
at 00:00:00.000, which reads as nothing happening. Same formula as the
clock path, on the position the host broadcasts.

Guarded on closing() rather than merely on the model existing: the
timing widget's timer keeps firing while a document is torn down and the
base scenario goes first, which crashed on closing a terminal.

Also let a page be told things. A wasm build has no command line, so its
query string is the only way in: ?network-join=host:port&network-terminal
reaches score as the matching arguments, and every option works, not
just those. That does mean a link can ask a page to do whatever a
command line could -- a deployment that is not yours is not one to open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The inspector's port combo boxes each asked one question -- which
devices can be a texture source, which are MIDI inputs -- and each asked
it with a lambda casting to a plug-in's own C++ type. That answer only
exists where the device object does, so a document whose score runs on
another machine had nothing to offer: no device objects, empty combo
box, no way to set a texture inlet at all.

DeviceKind is the same question asked of the device, which makes it a
fact the machine that has it can report. The four predicates become four
kinds, and each device answers for itself -- MIDI knows its direction
from its own settings, which is exactly why a cast could never tell.

DeviceDocumentPlugin holds what the other machine reported, and
makeDeviceCombo falls back to it when nothing local answers. Empty for
an ordinary document, so those behave exactly as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A device that fails to instantiate calls score::warning, and the GUI
fixture has a main window, so that is a modal nobody will dismiss and
the test hung. Devices are not what it checks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Document::environment() is created lazily and *replaced* once a session
knows where the files really are. The panel is told the document exists
during loadDocument, before that replacement, so the model captured a
reference to the local environment which setEnvironment then destroyed.
Calling list() through it is a dead vtable: in the wasm build that is
"RuntimeError: function signature mismatch" from
RemoteFileSystemModel::fetchMore, which is the reported hang when
opening the user library panel.

The model now takes how to reach the environment rather than the
environment, which also made it testable: the test replaces one between
construction and the first listing, exactly as a session does.

Third time this session that something read state during loadDocument
which is only settled afterwards -- the others were Document::environment
itself and the library import.

Also add --logs to headless-check.js: score's own output reaches the
browser console but was only printed when a run failed, which makes
"did it do the thing?" unanswerable on a successful run. It is how this
was found.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The option value was passed straight to QJSEngine::evaluate, so a path was
evaluated as an expression -- it failed, the result was discarded, and
--script did nothing without saying so. Reading the file when the value
names one makes the option usable for scripted and headless runs, which is
what it is for; logging the error means the next failure is visible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
GUIApplicationContext::panel<T> asserts rather than returning null, so a
score started with --no-gui died as soon as the library scan reached any
.mjs -- on a worker-dispatched callback, before it could serve anything.
A headless host is the normal way to run one half of a networked session.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
jcelerier and others added 28 commits August 17, 2026 21:05
The dialog groups this machine's hardware under a heading per enumerator --
"Cameras", "Screens". A terminal lists the peer's instead, and the category
was pasted onto the front of each name, so what arrived was one flat list of
"Cameras / Logitech BRIO" strings that could not be grouped or sorted.

DeviceCatalog::OnDevice now takes (category, name, settings) and the dialog
builds the same headings it builds for local hardware. The wire format
already carried it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Setting a value in the tree has always meant "send it to the device", and
NodeUpdateProxy did nothing at all when the device list had no such name --
which is every device on a terminal. A sink on the plugin gives that case
somewhere to go; nothing installs one in an ordinary document, so the local
path is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The category test asked every protocol on the machine to enumerate, which
pulls in the ones that discover over the network. Their enumerators start
threads that do not survive being created once per test case in a single
process: the binary segfaulted in DNSSDEnumerator::start on a later boot,
while the case passed when run alone.

The probe protocol now offers two devices under one heading, so the test can
tell a category from a name without depending on what is plugged in.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A process loaded from JSON has its ports rebuilt as real objects and removed
from the payload. serialize_impl only wrote them back in the JSON branch, so
the binary format lost them -- and score writes binary constantly: every
autosave, every crash backup, and moving an interval, which serialises its
processes and rebuilds them from those bytes.

The loss was silent and unrecoverable. After one drag the stand-in had no
ports, every cable into it dangled, and saving to .score emitted the absent
plug-in's uuid with no Inlets/Outlets -- so the machine that does have the
plug-in got a process stripped of everything, which is worse than not
opening the document.

The binary blob now carries a flag saying whether the ports are its own or
the payload's, written before the payload since that takes the rest of the
stream.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Two faults in the same listing path, both from the answer arriving later than
the request:

The add-device dialog handed the catalog a callback capturing a raw this.
DeviceCatalog::enumerate says outright that it may call back after it
returns, and the reply has up to the RPC timeout to arrive -- so cancelling
the dialog while a peer was slow wrote into a destroyed widget. It also had
no notion of which protocol it was answering for, so picking one protocol and
then another appended the first one's devices to the second one's list, and
selecting one handed over settings for a protocol nobody chose.

The remote file model inserted an empty range for an empty folder:
beginInsertRows(0, -1) is last < first, which asserts on a debug Qt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
request_stop and request_stop_interval asked whether the *focused* document
executes here. There is one execution controller per application and what is
playing belongs to whichever document started it, so selecting a terminal
left a running score with no way to stop it -- and a score that ended on its
own never cleared its transport state, since the finished handler goes
through the same call.

They now ask whether anything is executing at all. The play requests keep the
role guard: those are the ones that must not start a score on a machine that
is only watching one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
checkDelimiter breakpoints before it throws. That is right for a corrupt
file -- a bug in this build, worth stopping on -- but bytes off a socket are
not: a peer running a different version, or a truncated frame, is an ordinary
event. On a debug build any peer could stop the process with one bad frame,
before the catch that was there to handle it could run.

score::readingUntrustedData() says which of the two is being read; the
network add-on sets it around every deserialization of wire data.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Two tests passed with the thing they named deleted:

"An unclaimed process still gets a layer" never asked for the layer of a
stand-in -- only that a plain process does not get the fallback, and that a
lookup by key does not find it. Deleting the routing in findDefaultFactory
left it green.

"Scripting is off unless asked for" checked four settings defaults. A default
is not an enforcement: deleting the allowScripting guard, which is what stops
a remote client evaluating JavaScript in this process, left every remote
control test passing. It now connects, sends a Console message and looks at
whether the console ran it -- with the setting off and then on, since a test
for the refusal alone would also pass against a Console handler that was
simply broken.

That needs the panel, so the file moves to the GUI fixture; and the handler
went through panel<T>, which aborts without a GUI, so a headless instance
with scripting enabled died on the first script. It uses findPanel now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Everything carrying a host's device state to a peer -- the broadcast on
change, the one on a client joining, and the query at join -- could be
deleted with every terminal test still passing. The reason was the fixture:
ProbeProtocolFactory deliberately builds no device, so no test host owned a
DeviceInterface for any of it to fire on, and the tests set the terminal's
copy by hand.

ConnectedProbeProtocolFactory owns one, with connected() and kinds() a test
can drive. The new case joins a session where the device already exists,
checks the terminal learned its state and both its kinds without holding any
device itself, then changes the state on the host and checks it follows.
Deleting any of the three sites fails it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Code review rejects multi-paragraph explanations of why a change was made;
that belongs in the commit message. First pass over the worst offenders.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Down to short what-comments plus API doc lines. The reasoning behind each
change is in its commit message.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
DeviceExplorerModel gains replaceDevice, and DeviceDocumentPlugin a signal for
"this device's own tree changed". The signal is driven by the model's own
structural changes rather than by instrumenting each mutator, so anything that
adds or removes a node -- a refresh, OSCQuery discovery, learning -- reports
itself. Value updates deliberately do not: a value arriving is not the tree
changing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
releaseThread() ran quit() then wait() over every started thread once
nothing was in flight. Both media layers release from their destructor,
on the UI thread, so the UI thread waited there for work it was itself
responsible for letting run. In a browser that wait never ends: a worker
only finishes starting once the main thread returns to the event loop.
Adding a sound or a video to a session froze the tab for good.

The threads now outlive every release and go away with the pool, which
is hooked to the application ending -- a worker whose event loop is
still running once QCoreApplication is gone warns and has nothing left
to run on.

Two more things found underneath:

  - acquireThread() started every thread it would ever hand out on the
    first call: hardware_concurrency()/2, 24 here, each with the process
    stack size, to answer one waveform. Started as handed out now, and
    capped at 2 on wasm, where creating one past the browser's pool has
    to return to the event loop.
  - every field had two writers (acquire from the UI thread, release
    from whoever held the last reference) and no lock. A mutex covers
    them; these are called once per media layer, not per frame.

test_unit_thread_pool asserts the release does not wait for work in
flight, and that one acquisition starts one thread.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
paint_impl called recompute() whenever it had nothing to draw, and
recompute() gives up without marking itself done on five conditions: no
data, width or height under 2, no zoom, no sample rate, no view. So the
two called each other for as long as the process existed -- which is
what a file that lives on another machine does here.

Of those five, only the view has no signal to re-ask on, and painting is
precisely the proof that one exists: everything else (setData,
on_finishedDecoding, heightChanged, widthChanged, zoom, tempo, colours)
already calls recompute() when it changes. So paint asks once, and
duplicates none of the conditions -- an earlier version of this fix
repeated two of them in the caller, which rots as soon as a sixth is
added.

Also fixes the constructor dereferencing a null parent, which is latent
in production (a layer always has one) and showed up under test.

test_integration_sound_view_spin paints 50 times and asserts the count
grows by at most one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Environment::list answers for one directory, which is the right
primitive and never what a caller looking for presets or library files
wants. listRecursive walks it: directories that cannot be listed are
skipped rather than failing the whole walk, the depth is bounded (both
the round trips and a symlink pointing at its own parent), and the
result arrives once, at the end.

It lives next to Environment because presets, library files and the
media cache all need the same thing.

maxInlineTransferBytes() gives the 8 MB ceiling one home: the host's
fs.* refused anything larger by its own constant, and callers about to
send bytes have to know the same number.

test_unit_environment_walk covers it against an environment that answers
out of order, which is what a network gives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
initPresets scanned <library>/packages off QSettings with a
RecursiveWatch, and selecting one opened the file. A terminal's library
folder is empty, or in a browser does not exist at all, so it offered no
presets.

It walks <LIBRARY>:packages through the environment now and reads the
chosen one the same way. The tree item holds the URI rather than a path,
since a path from the other machine names nothing here; reading is
asynchronous, so applying a preset splits out into applyPreset.

Note the dialog's own QSettings fallback to Documents/<org>/<app> is
gone: the library settings model writes that same default at
construction, and duplicating it was the bug-shaped part. A build
without score-plugin-library now lists no presets.

test_integration_device_preset_source drives the dialog against a
scripted non-local environment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Dropping a file named it by a path, and a path only means something on
the machine holding it. The command replicated, the host created the
process -- pointing at /score/imports/kick.wav in a browser's memory, or
at a laptop's home directory. Nothing could open it, so the drop looked
like it had done nothing at all.

score::importFile puts the bytes in the media cache under
<sha1-16>-<name>, and sends them on when the score runs elsewhere.
Content-addressed because that is what makes the two machines agree:
each keeps its own copy and <CACHE>:<entry> names the same bytes on
both, which is also what the document stores. Over the inline limit it
refuses outright -- a process naming a file the other machine will never
have is worse than an import that visibly does nothing.

The wasm-only staging block in getDrop becomes a decision, since there
are two reasons not to use a file where it lies and they end the same
way: the source is about to vanish (a browser), or the machine that will
open it is not this one. A desktop editing its own document still uses
the file where it is.

The file picker had the same bug by the same route: openFileToImport and
openFilesToImport now take the document context -- all four call sites
already had one -- and go through importPickedFile.

test_unit_import_file and test_integration_terminal_file_drop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The preview was guarded by QFile::exists on the library entry's path.
On a terminal the library is the other machine's, so that is never true
and the panel showed nothing.

The panel keeps the fast path for a file it has and otherwise reads
through the environment. previewWidget gains an overload taking the
contents, defaulting to ignoring them and delegating to the path
version, so every other implementor is untouched; Filter and VSA
override it, CSF has no preview at all and needs nothing. The path is
still passed, since the extension chooses the parser and names the
.vert companion.

Little of this is new machinery: programFromISFFragmentShaderPath and
its VSA counterpart already took the bytes and only opened the file when
given none.

m_awaitedPreview drops an answer that arrives after the selection has
moved on, which is otherwise a preview of the wrong file.

Not covered by a test: whether a preview actually renders. The shader
sweeps need a GPU, so an assertion written without one would only be
testing that nothing threw.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Reported as "I cannot add any Evdev device I/O from the terminal", and it
was never about evdev: no device of a protocol this build lacks could be
added at all. Which is the ordinary case on a terminal -- the score runs
on the other machine, and that is where the evdev, the MIDI ports and
the cameras are.

Three gates, each enough on its own to stop it:

  - getSettings() returned an empty DeviceSettings whenever there was no
    protocol widget, and there is none for a protocol we have no factory
    for -- the form is C++ in a plug-in we do not have. So what the host
    enumerated was dropped on the floor. The dialog keeps it now,
    whether it came from the hardware list, a preset or setSettings.
  - checkDeviceInstantiatable asked *this* machine for the factory and
    refused when there was none, which disabled the Add button. It
    accepts a protocol the catalog offers now, judging only that the
    name is free: whether two devices of a protocol can coexist is that
    protocol's own rule, and it lives on the other machine.
  - the devices column was shown before any answer arrived and never
    hidden, so a protocol with nothing plugged into it kept whatever the
    previous one had shown. Hidden on every protocol change, shown when
    the first device arrives.

DeviceCatalog::Protocol::constructible already described this in its
comment -- "such a protocol can only be used through what it
enumerates". It just was not implemented anywhere.

test_integration_terminal_device_add covers the three; each fails alone
when reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Add became clickable for a protocol only the host has, and then did
nothing. getSettings() was fixed to keep what the other machine sent;
the add path reads getDevice(), which had the same hole one function
over and still answered with an empty node. Its caller then read the
settings out of that node through a null target on its way to adding
nothing.

getDevice() now builds the device from what was chosen -- the enumerated
hardware or a preset -- when there is no widget to read it from, and the
caller copes with an empty node rather than dereferencing it.

Found on the way, and unrelated to terminals: createDeviceFromNode
called makeDevice through a null protocol factory. Any document naming
a protocol this build has no factory for crashed there. The node is kept
now, as loading one already did. A terminal never reached it -- it
returns before instantiating anything -- which is why this survived.

test_integration_terminal_device_add covers the add path;
test_network_remote_enumeration checks such a device actually reaches
the machine running the score, which is where it gets made.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A machine that has no factory for a protocol never decodes its settings:
it keeps the bytes and hands them back when the device is added, and the
machine that does have the protocol reconstructs them. That is the only
way a terminal adds a device, and it aborted the host.

The protocol writes its settings into the same JSON object as the
device's own Name and Protocol, and some protocols -- evdev among them
-- call a setting of their own "Name" too. Building the payload skips
every member score owns, so that writing it back cannot duplicate them,
and that takes the protocol's "Name" with it. Reading then asks for a
member that is not there: rapidjson asserts, and the host dies on a
device the terminal sent.

The members score owns are put back before the protocol reads, which
hands it the object it originally wrote.

test_integration_opaque_device_settings carries one enumerated device
per protocol the way a machine without the factory does, and compares
what the protocol writes before and after. It aborts on Evdev without
this, which is the reported crash.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A device added from a terminal arrives on the host as a command carrying
the node the terminal knew: a name, a protocol, and nothing inside. The
host then opens the thing and finds out what it contains -- a mouse has
axes and buttons, a MIDI port has its channels -- and fills the node
before inserting it.

That insertion is at the root, and root insertions were deliberately not
announced: "a device appearing, which its own command already carries".
True when a document is loaded, and false here, because the command was
written by a machine that cannot even make the device. So the terminal
showed the device with nothing under it, permanently.

A device appearing now announces its tree like any other change. The
broadcaster already coalesces, so a refresh that arrives as many
insertions still sends one tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A peer could already ask for a value: it travels to the machine with the
device, which performs it. The other direction did not exist. A mouse
moving, a fader being touched, an OSC device answering -- all of that
happens where the devices are, and a terminal has nothing to hear it
from. Its tree showed names and the values they had when it joined.

on_valueUpdated is where a device reports, so it gains an observer
alongside the existing sink. The two are deliberately separate: a sink
is "somebody asked for this, carry it out", an observer is "this is what
happened here".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
Two harness faults, both of which hid real coverage.

The three shader sweeps have been reported as failures for a long time
and were never running. Not the GPU and not the display: the fixture
gives every test its own XDG_CONFIG_HOME and application name, on
purpose, so the library root a user configured is deliberately invisible
and the sweep skipped for want of one. Catch2 exits 4 when everything
skips, and ctest reports that as a failure. Now the library is probed at
configure time and passed through the test environment, and a skip is
declared as a skip. All three pass over ~2000 shaders.

score::test::absent_factory takes a factory out of the application's
list for a scope and puts it back. The factory lists are public maps, so
this needs nothing from production code. It exists because the session
tests run both peers in one process sharing one set of factories, and so
could not express the case that matters most on a terminal: a peer that
cannot construct what the other side can. The asymmetry a wire needs is
temporal -- write while it is hidden, read with it present -- which is
exactly a terminal-to-host exchange.

test_integration_opaque_device_settings now reproduces the evdev abort
through the real code path. It previously had to hand-write the wire
byte by byte, because a process that has the factory re-encodes from
settings it never decoded and the carried bytes never travel.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
A terminal's console edits a score running on another machine. Document
edits happen to replicate, because they are commands, so the console
looks like it works -- and everything that is not a command does not.
Score.device("x") looks in a device list that is empty on a terminal and
always will be; execution is not running there; the hardware is
elsewhere. Forwarding one call at a time would mean keeping a second,
partial API in step with the real one.

So a document gains a script sink, the way it already has an
environment: set when the document is a view of a score running
somewhere else, and the console asks that machine instead of answering
from a document with nothing behind it.

The session layer has no business knowing what JavaScript is, so
score::ScriptEvaluator is declared here and the JS plug-in registers it.
A build without that plug-in registers nothing and says so, rather than
answering with a silence that reads like a script that printed nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
It was a function-local static holding a reference to one plug-in
instance's engine, published to a process-global pointer and never
withdrawn. Two ways that bites: after the plug-in is destroyed the
global points at an evaluator whose engine is gone, and in a process
that builds a second application -- which every test binary does, one
per test case -- the second construction republishes the *first*
evaluator, so a peer's script.eval runs against a destroyed QJSEngine.

Owned by the plug-in now, and unregistered in its destructor.

Found by an adversarial review of this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The preset scan replaced RecursiveWatch::scanAsync, which ran off the
UI thread, with listRecursive -- and a local environment answers a
listing inline, so the whole library was walked in one go before the
Add-device dialog appeared. That is a straight regression on desktop,
introduced while fixing the terminal case.

listRecursive now takes the object to yield to between directories: each
one is a turn of the event loop, and the walk stops if that object goes
away. A remote environment was already asynchronous and is unaffected.

The test waited on nothing and passed anyway, because constructing the
dialog happens to pump events -- it now waits for the walk, and checks
what was asked of the environment afterwards rather than before.

Found by an adversarial review of this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
The session work spans both repositories and they change together: the
add-on's remote-control branch will not build against master's score,
and master's add-on will not build against this. CI has to be told
which, or it tests a combination that exists nowhere.

To be dropped when both are merged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wXVcUPzvcDTVzLpWJzeqP
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.

1 participant