diff --git a/spec/smith/cmux_notify_spec.cr b/spec/smith/cmux_notify_spec.cr new file mode 100644 index 0000000..14fcfc5 --- /dev/null +++ b/spec/smith/cmux_notify_spec.cr @@ -0,0 +1,836 @@ +require "socket" +require "../spec_helper" +require "../../src/smith/cmux_client" +require "../../src/smith/notify" +require "../../src/smith/turn_notifier" +require "../../src/smith/agent" +require "../../src/smith/tools" +require "../../src/smith/cli" +require "../../src/smith/session" + +# Records what `Smith::Notify` hands over, so the payload rules can be asserted +# without a cmux daemon — which is the point: nothing here should depend on +# there being one. +# No `CMUX_*` at all: the shape of a shell that is not inside cmux. +private def no_cmux_env : Hash(String, String?) + {} of String => String? +end + +# A config that says everything, so "the environment wins" and "the config is +# left alone" are both observable against it. +private def filled_config : Smith::NotifyConfig + Smith::NotifyConfig.new( + enabled: true, + socket_path: "/config/cmux.sock", + surface_id: "config-surface", + workspace_id: "config-workspace" + ) +end + +private class RecordingClient < Smith::CmuxClientable + getter payloads = [] of Hash(String, JSON::Any) + property result : Bool = true + property available : Bool = true + + def notify(payload : Hash(String, JSON::Any)) : Bool + @payloads << payload + @result + end + + def available? : Bool + @available + end + + def last : Hash(String, JSON::Any) + @payloads.last + end +end + +# The client a socket implementation will eventually be: one that finds out +# something is wrong only when it tries. A notification is the last thing a +# run does, so whatever this throws must stay inside the notification. +private class ExplodingClient < Smith::CmuxClientable + def notify(payload : Hash(String, JSON::Any)) : Bool + raise Socket::ConnectError.new("Connection refused") + end + + def available? : Bool + true + end +end + +describe Smith::NotifyConfig do + it "says nothing at all, and delivers nothing, until something does" do + config = Smith::NotifyConfig.new + + # `enabled` is tri-state on purpose: nil is "nobody said", which is the + # answer a terminal is allowed to overrule. false would be somebody + # turning notifications off, and no environment may overrule that. + config.enabled.should be_nil + config.enabled?.should be_false + config.socket_path.should be_nil + config.surface_id.should be_nil + config.workspace_id.should be_nil + config.socket?.should be_false + config.deliverable?.should be_false + end + + it "is deliverable only when enabled and holding a socket path" do + Smith::NotifyConfig.new(enabled: true).deliverable?.should be_false + Smith::NotifyConfig.new(socket_path: "/tmp/cmux.sock").deliverable?.should be_false + Smith::NotifyConfig.new(enabled: true, socket_path: "/tmp/cmux.sock").deliverable?.should be_true + end + + it "is not deliverable when explicitly turned off, however much else is there" do + Smith::NotifyConfig.new(enabled: false, socket_path: "/tmp/cmux.sock").deliverable?.should be_false + end + + it "defaults the timeout to something a socket can live with" do + Smith::NotifyConfig.new.timeout.should eq(1.0) + end + + describe ".from_table" do + it "reads the [notify] keys" do + table = TOML.parse(<<-TOML) + enabled = true + socket_path = "/tmp/cmux.sock" + surface_id = "surface:1" + workspace_id = "workspace:1" + timeout = 2.5 + TOML + + config = Smith::NotifyConfig.from_table(table) + + config.enabled.should be_true + config.socket_path.should eq("/tmp/cmux.sock") + config.surface_id.should eq("surface:1") + config.workspace_id.should eq("workspace:1") + config.timeout.should eq(2.5) + end + + it "says nothing when there is no section at all" do + config = Smith::NotifyConfig.from_table(nil) + + config.enabled.should be_nil + config.socket_path.should be_nil + config.deliverable?.should be_false + end + + it "reads an integer timeout as well as a float one" do + table = TOML.parse("timeout = 3") + + Smith::NotifyConfig.from_table(table).timeout.should eq(3.0) + end + + it "treats a timeout that could not work as the default" do + Smith::NotifyConfig.from_table(TOML.parse("timeout = 0")).timeout.should eq(1.0) + Smith::NotifyConfig.from_table(TOML.parse("timeout = -1.5")).timeout.should eq(1.0) + end + + it "turns blank strings into unset, so they cannot shadow the environment" do + table = TOML.parse(<<-TOML) + enabled = true + socket_path = " " + surface_id = "" + TOML + + config = Smith::NotifyConfig.from_table(table) + + config.socket_path.should be_nil + config.surface_id.should be_nil + config.socket?.should be_false + end + + it "ignores a value of the wrong type instead of raising" do + # A config file is written by a human, and `socket_path = true` is a + # typo, not a reason for smith to refuse to start. + table = TOML.parse(<<-TOML) + enabled = "yes" + socket_path = 42 + TOML + + config = Smith::NotifyConfig.from_table(table) + + # Not false: an unreadable `enabled` is nobody having said, so the + # terminal keeps its say rather than a typo switching notifications off. + config.enabled.should be_nil + config.socket_path.should be_nil + end + + it "trims the values it does keep" do + table = TOML.parse(<<-TOML) + socket_path = " /tmp/cmux.sock " + surface_id = " surface:1 " + TOML + + config = Smith::NotifyConfig.from_table(table) + + config.socket_path.should eq("/tmp/cmux.sock") + config.surface_id.should eq("surface:1") + end + end +end + +describe Smith::CmuxClient do + describe ".resolve" do + it "leaves the config alone when the environment says nothing" do + resolved = Smith::CmuxClient.resolve(filled_config, no_cmux_env) + + resolved.enabled?.should be_true + resolved.socket_path.should eq("/config/cmux.sock") + resolved.surface_id.should eq("config-surface") + resolved.workspace_id.should eq("config-workspace") + end + + it "prefers the environment: it describes the terminal that is open now" do + env = { + "CMUX_SOCKET_PATH" => "/env/cmux.sock", + "CMUX_SURFACE_ID" => "env-surface", + "CMUX_WORKSPACE_ID" => "env-workspace", + } of String => String? + + resolved = Smith::CmuxClient.resolve(filled_config, env) + + resolved.socket_path.should eq("/env/cmux.sock") + resolved.surface_id.should eq("env-surface") + resolved.workspace_id.should eq("env-workspace") + end + + it "switches notifications on inside cmux, without a config file asking" do + # cmux exports a socket path into every process it spawns; that is how + # smith knows this session is running inside one. There is no `CMUX=1` + # flag to read, and the terminal's own environment is checked below. + silent = Smith::NotifyConfig.new + + Smith::CmuxClient.resolve(silent, {"CMUX_SOCKET_PATH" => "/cmux.sock"}).enabled?.should be_true + end + + it "leaves notifications off in a shell that is not cmux" do + # The default has to be "nothing happens" — a plain terminal session + # must not be changed by this feature, and must not spend a turn + # looking for a socket that was never there. + Smith::CmuxClient.resolve(Smith::NotifyConfig.new, no_cmux_env).enabled?.should be_false + end + + it "honours an explicit enabled = false over the terminal" do + # Somebody turned them off. Being inside cmux is not a reason to + # overrule that, which is why `false` is not folded into "unset". + off = Smith::NotifyConfig.new(enabled: false) + + resolved = Smith::CmuxClient.resolve(off, {"CMUX_SOCKET_PATH" => "/cmux.sock"}) + + resolved.enabled?.should be_false + resolved.deliverable?.should be_false + end + + it "does not read a socket path from a config file as being inside cmux" do + # Naming a location is not the same statement as the terminal making + # itself known: a config file can point at a socket for a session that + # is not running inside cmux at all. Left to `enabled` to ask for. + configured = Smith::NotifyConfig.new(socket_path: "/config/cmux.sock") + + Smith::CmuxClient.resolve(configured, no_cmux_env).enabled?.should be_false + end + + it "reads a falsey socket variable as cmux not being there" do + # One rule for every variable: a falsey value is an absent value, so it + # cannot switch notifications on either. This is not a hypothetical — + # cmux exports `CMUX_SOCKET=` empty alongside a populated + # `CMUX_SOCKET_PATH` in the same shell. + %w[0 false no off].each do |value| + {"CMUX_SOCKET_PATH" => value, "CMUX_SOCKET_PATH" => value.upcase}.each do |key, spelling| + env = {key => spelling} of String => String? + Smith::CmuxClient.resolve(Smith::NotifyConfig.new, env).enabled?.should be_false, "#{key}=#{spelling}" + end + end + end + + it "reads an empty socket variable as unset rather than as a location" do + # `export CMUX_SOCKET=` and no export at all are the same statement. + # Read as a location it would shadow the real one with nothing. + env = {"CMUX_SOCKET" => ""} of String => String? + + resolved = Smith::CmuxClient.resolve(filled_config, env) + + resolved.socket_path.should eq("/config/cmux.sock") + resolved.deliverable?.should be_true + end + + it "keeps the timeout: it is not something an environment describes" do + env = {"CMUX_SOCKET_PATH" => "/env/cmux.sock"} of String => String? + timed = Smith::NotifyConfig.new(timeout: 4.0) + + Smith::CmuxClient.resolve(timed, env).timeout.should eq(4.0) + end + + describe "socket path priority" do + it "takes CMUX_SOCKET_PATH over the older spellings" do + env = { + "CMUX_SOCKET_PATH" => "/canonical.sock", + "CMUX_SOCKET" => "/older.sock", + "CMUX" => "/oldest.sock", + } of String => String? + + Smith::CmuxClient.resolve(filled_config, env).socket_path.should eq("/canonical.sock") + end + + it "falls back to CMUX_SOCKET, then to CMUX when it looks like a path" do + Smith::CmuxClient.resolve(filled_config, {"CMUX_SOCKET" => "/older.sock", "CMUX" => "/oldest.sock"} of String => String?).socket_path.should eq("/older.sock") + Smith::CmuxClient.resolve(filled_config, {"CMUX" => "/tmp/cmux.sock"} of String => String?).socket_path.should eq("/tmp/cmux.sock") + end + + it "does not read a flag-shaped CMUX as a location" do + # `CMUX=1` is the shape of a flag, not of a path. Treating `1` as one + # would connect to a file called `1` in the current directory. + silent = Smith::NotifyConfig.new + resolved = Smith::CmuxClient.resolve(silent, {"CMUX" => "1"} of String => String?) + + resolved.socket_path.should be_nil + # …and a flag is not a socket, so it cannot switch notifications on + # either: nothing in this environment says "you are inside cmux". + resolved.enabled?.should be_false + end + + it "skips a falsey socket variable and keeps looking" do + env = { + "CMUX_SOCKET_PATH" => "", + "CMUX_SOCKET" => "0", + "CMUX" => "/tmp/cmux.sock", + } of String => String? + + Smith::CmuxClient.resolve(filled_config, env).socket_path.should eq("/tmp/cmux.sock") + end + + it "falls through to the config when every variable is falsey" do + env = { + "CMUX_SOCKET_PATH" => " ", + "CMUX_SOCKET" => "off", + } of String => String? + + Smith::CmuxClient.resolve(filled_config, env).socket_path.should eq("/config/cmux.sock") + end + + it "leaves a whitespace surface or workspace id unset" do + env = { + "CMUX_SURFACE_ID" => " ", + "CMUX_WORKSPACE_ID" => "", + } of String => String? + + resolved = Smith::CmuxClient.resolve(filled_config, env) + + resolved.surface_id.should eq("config-surface") + resolved.workspace_id.should eq("config-workspace") + end + + it "takes a documented spelling when the exported one is missing" do + # cmux documents `CMUX_TAB_ID` and `CMUX_PANEL_ID` and exports + # `CMUX_WORKSPACE_ID`/`CMUX_SURFACE_ID` carrying the same two values. + # Which pair a build offers is not something smith can ask about, so + # neither is assumed. + silent = Smith::NotifyConfig.new + env = {"CMUX_TAB_ID" => "tab-1", "CMUX_PANEL_ID" => "panel-1"} of String => String? + + resolved = Smith::CmuxClient.resolve(silent, env) + + resolved.workspace_id.should eq("tab-1") + resolved.surface_id.should eq("panel-1") + end + + it "prefers the workspace and surface ids it was documented with" do + env = { + "CMUX_WORKSPACE_ID" => "workspace-1", + "CMUX_TAB_ID" => "tab-1", + "CMUX_SURFACE_ID" => "surface-1", + "CMUX_PANEL_ID" => "panel-1", + } of String => String? + + resolved = Smith::CmuxClient.resolve(Smith::NotifyConfig.new, env) + + resolved.workspace_id.should eq("workspace-1") + resolved.surface_id.should eq("surface-1") + end + end + end + + describe ".build" do + it "resolves and then builds, in one step" do + client = Smith::CmuxClient.build( + Smith::NotifyConfig.new(enabled: true), + {"CMUX_SOCKET_PATH" => "/tmp/cmux.sock"} of String => String? + ) + + client.should be_a(Smith::CmuxClientable) + end + end + + describe ".client" do + it "is null when notifications are off" do + Smith::CmuxClient.client(Smith::NotifyConfig.new(socket_path: "/tmp/cmux.sock")).should be_a(Smith::NullCmuxClient) + end + + it "is null when there is no socket to talk to" do + Smith::CmuxClient.client(Smith::NotifyConfig.new(enabled: true)).should be_a(Smith::NullCmuxClient) + end + + it "is null even when everything is in place, until the wire exists" do + # The seam this PR builds stops here on purpose. When a real client + # arrives, this is the expectation that changes — and it should change + # loudly, not by a spec quietly going green. + config = Smith::NotifyConfig.new(enabled: true, socket_path: "/tmp/cmux.sock") + + Smith::CmuxClient.client(config).should be_a(Smith::NullCmuxClient) + end + end +end + +describe Smith::NullCmuxClient do + it "accepts nothing and delivers nothing" do + client = Smith::NullCmuxClient.new + + client.available?.should be_false + client.notify(Hash(String, JSON::Any).new).should be_false + end +end + +describe Smith::Notify do + it "names what kind of message it is sending" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done") + + client.last["type"].as_s.should eq("notification") + end + + it "always sends a title, even an empty one" do + client = RecordingClient.new + Smith::Notify.new(client).notify("") + + client.last["title"].as_s.should eq("") + end + + it "sends subtitle and body when they say something" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", subtitle: "2 failed", body: "spec/smith/notify_spec.cr") + + client.last["subtitle"].as_s.should eq("2 failed") + client.last["body"].as_s.should eq("spec/smith/notify_spec.cr") + end + + it "leaves an absent subtitle or body out of the payload entirely" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done") + + client.last.has_key?("subtitle").should be_false + client.last.has_key?("body").should be_false + end + + it "leaves a blank body out, because an empty one renders as a gap" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", subtitle: " ", body: "") + + client.last.has_key?("subtitle").should be_false + client.last.has_key?("body").should be_false + end + + it "trims what it does send" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", body: " all green ") + + client.last["body"].as_s.should eq("all green") + end + + it "carries extra fields into the payload" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", priority: "high", count: 3, ratio: 0.5, urgent: true, missing: nil) + + client.last["priority"].as_s.should eq("high") + client.last["count"].as_i.should eq(3) + client.last["ratio"].as_f.should eq(0.5) + client.last["urgent"].as_bool.should be_true + client.last["missing"].raw.should be_nil + end + + it "carries nested extras, widening the numbers JSON wants" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", tags: ["ci", "main"], detail: {"file" => "notify.cr", "line" => 12}) + + client.last["tags"].as_a.map(&.as_s).should eq(["ci", "main"]) + client.last["detail"]["file"].as_s.should eq("notify.cr") + client.last["detail"]["line"].as_i.should eq(12) + end + + it "lets an extra win over the field notify would have filled in" do + # The caller knows better than the default here: a `type` of something + # other than "notification" is a deliberate choice, not a collision. + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", type: "status") + + client.last["type"].as_s.should eq("status") + client.last["title"].as_s.should eq("Build done") + end + + it "reports what the client reported" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done").should be_true + + client.result = false + Smith::Notify.new(client).notify("Build done").should be_false + end + + it "reports unavailable, and sends nothing, through a null client" do + notify = Smith::Notify.new(Smith::NullCmuxClient.new) + + notify.enabled?.should be_false + notify.notify("Build done").should be_false + end + + it "reports available when the client has somewhere to deliver to" do + client = RecordingClient.new + Smith::Notify.new(client).enabled?.should be_true + + client.available = false + Smith::Notify.new(client).enabled?.should be_false + end + + it "swallows a client that throws, because a notification must not end a run" do + # The socket can vanish between the availability check and the write, and + # cmux can be killed mid-run. Both are ordinary, and neither is worth the + # turn that was just completed. + Smith::Notify.new(ExplodingClient.new).notify("Build done").should be_false + end + + it "serialises to the JSON a daemon would receive" do + client = RecordingClient.new + Smith::Notify.new(client).notify("Build done", subtitle: "2 failed") + + client.last.to_json.should eq(%({"type":"notification","title":"Build done","subtitle":"2 failed"})) + end +end + +# What the first consumer does with a run. Driven through the real agent loop +# wherever a loop exists to drive, because the point of this listener is that +# it reacts to events as they arrive — a hand-written sequence would pass +# whatever order it happened to assert. +private class NotifyingProvider < Smith::LLM::Provider + getter calls = 0 + + def name : String + "mock" + end + + def default_model : String + "mock-model" + end + + def complete(request : Smith::LLM::Request) : Smith::LLM::Response + @calls += 1 + + if @calls == 1 + # Turn one announces and calls a tool — the announcement is not the + # answer, and the run is not over. + blocks = [ + Smith::LLM::ContentBlock.text("Let me look at that."), + Smith::LLM::ContentBlock.tool_use("call_1", "read_file", JSON.parse(%({"path": "spec/spec_helper.cr"}))), + ] + else + blocks = [ + Smith::LLM::ContentBlock.text("Done. The tests pass."), + ] + end + + Smith::LLM::Response.new("resp_#{@calls}", request.model, blocks, usage: Smith::LLM::Usage.new(10, 5, 15)) + end +end + +describe Smith::TurnNotifier do + it "says nothing until the turn is over" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::AssistantText.new("thinking out loud")) + client.payloads.should be_empty + + notifier.handle(Smith::Events::TurnCompleted.new(1)) + client.payloads.size.should eq(1) + end + + it "names the run and carries where it came from" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client), subtitle: "smith · notify") + + notifier.handle(Smith::Events::AssistantText.new("all green")) + notifier.handle(Smith::Events::TurnCompleted.new(3)) + + payload = client.last + payload["title"].should eq("Smith") + payload["subtitle"].should eq("smith · notify") + payload["body"].should eq("all green") + end + + it "reports the answer, not the announcement that came before the tools" do + # What a model says before calling a tool is a promise of work, not a + # result. Sent as the body it would read, an hour later, as though the run + # had stopped mid-sentence. + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::AssistantText.new("Let me look at that.")) + notifier.handle(Smith::Events::ToolStart.new("call_1", "read_file", JSON.parse("{}"))) + notifier.handle(Smith::Events::ToolFinished.new("call_1", "read_file", "contents", false)) + notifier.handle(Smith::Events::AssistantText.new("Done.")) + notifier.handle(Smith::Events::TurnCompleted.new(2)) + + client.last["body"].should eq("Done.") + end + + it "sends no body at all for a run that ended among its tools" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::AssistantText.new("Let me look at that.")) + notifier.handle(Smith::Events::ToolStart.new("call_1", "read_file", JSON.parse("{}"))) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + + client.last.has_key?("body").should be_false + end + + it "collapses a multi-paragraph answer into one line" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::AssistantText.new("First paragraph.\n\nSecond one,\twith a tab.")) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + + client.last["body"].should eq("First paragraph. Second one, with a tab.") + end + + it "cuts a long answer at a word boundary and says it did" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + long = Array.new(40) { |i| "word#{i}" }.join(" ") + notifier.handle(Smith::Events::AssistantText.new(long)) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + + body = client.last["body"].as_s + body.size.should be <= Smith::TurnNotifier::MAX_BODY + 1 + body.ends_with?("…").should be_true + + # The cut happened at a space rather than inside a word: what is left is + # one whole token, and the source had none of any other shape. + kept = body[0, body.size - 1] + kept.split(" ").last.should match(/^word\d+$/) + # …and it dropped something rather than merely trailing off. + kept.split(" ").size.should be < long.split(" ").size + end + + it "counts characters rather than bytes, so a body cannot be cut in half" do + # German text is where this shows: a byte-oriented cut would split a + # two-byte character and send an invalid string. + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + long = "Grüße " * 60 + notifier.handle(Smith::Events::AssistantText.new(long)) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + + body = client.last["body"].as_s + body.size.should be <= Smith::TurnNotifier::MAX_BODY + 1 + body.valid_encoding?.should be_true + end + + it "leaves an answer that fits alone, ellipsis and all" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::AssistantText.new("short")) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + + client.last["body"].should eq("short") + end + + it "notifies once per turn, and again for the next one" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + 2.times do |i| + notifier.handle(Smith::Events::AssistantText.new("turn #{i}")) + notifier.handle(Smith::Events::TurnCompleted.new(i + 1)) + end + + client.payloads.size.should eq(2) + client.payloads[0]["body"].should eq("turn 0") + client.payloads[1]["body"].should eq("turn 1") + end + + it "drops what a run said before it failed, so the next run starts clean" do + # A run does not have to end on a completed turn: a provider that fails, a + # budget that runs out and a window that fills each end one, and none of + # them is followed by `TurnCompleted`. Whatever was collected belongs to + # the run that died, and left standing it would be prefixed to the answer + # of the next one — a session of two turns would notify "second answer" + # with the first turn's half-answer glued in front of it. + [ + Smith::Events::TurnError.new("Provider completion failed"), + Smith::Events::BudgetExceeded.new(spent_usd: 2.0, limit_usd: 1.0), + Smith::Events::ContextExhausted.new(9000, 8000, 0), + ].each do |ending| + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::AssistantText.new("the run that died")) + notifier.handle(ending) + client.payloads.should be_empty, "#{ending.class} notified" + + notifier.handle(Smith::Events::AssistantText.new("the answer after it")) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + + client.last["body"].should eq("the answer after it"), "#{ending.class} leaked" + end + end + + it "ignores everything that is not a turn ending" do + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client)) + + notifier.handle(Smith::Events::ToolStart.new("call_1", "bash", JSON.parse("{}"))) + notifier.handle(Smith::Events::UsageUpdated.new(Smith::LLM::Usage.new(1, 1, 2))) + notifier.handle(Smith::Events::TurnError.new("provider said no")) + notifier.handle(Smith::Events::BudgetExceeded.new(spent_usd: 1.5, limit_usd: 1.0)) + + client.payloads.should be_empty + end + + it "says nothing through a null client, and survives a client that throws" do + # Not inside cmux is the ordinary case: a plain terminal run must not be + # changed by this feature, and must not pay for it either. + null_notifier = Smith::TurnNotifier.new(Smith::Notify.new(Smith::NullCmuxClient.new)) + null_notifier.handle(Smith::Events::TurnCompleted.new(1)) + + # A notification is the last thing a run does; it must not be the reason + # one ends. This call is the assertion: a client that throws out of + # `handle` fails the example right here. + exploding = Smith::TurnNotifier.new(Smith::Notify.new(ExplodingClient.new)) + exploding.handle(Smith::Events::TurnCompleted.new(1)) + end + + describe "through the real agent loop" do + it "is told by the events a run actually emits" do + provider = NotifyingProvider.new + registry = Smith::Tools::Registry.default + agent = Smith::Agent.new(provider: provider, registry: registry, model: "mock-model") + + client = RecordingClient.new + notifier = Smith::TurnNotifier.new(Smith::Notify.new(client), subtitle: "spec-project") + + # The same two lines `CLI#build_agent` runs, which is the wiring under + # test: a listener alongside the renderer rather than inside it. + agent.on_event { |event| notifier.handle(event) } + agent.send("Read spec_helper and tell me when the tests pass") + + # One notification for the whole run — the intermediate turn that called + # a tool announced itself and was not over. + client.payloads.size.should eq(1) + payload = client.last + payload["type"].should eq("notification") + payload["title"].should eq("Smith") + payload["subtitle"].should eq("spec-project") + payload["body"].should eq("Done. The tests pass.") + end + end +end + +# The wiring itself: what `CLI#build_agent` attaches, and what a resolved +# notification says about where the run is. Reaching into the private helpers +# rather than restating them is the same reason clear_persist_spec.cr does — +# a copy written out in the spec would pass whatever the CLI happened to do. +class Smith::CLI + def notify_for_spec : Smith::Notify + notify + end + + def notify_subtitle_for_spec(session : Smith::Session::Data?) : String? + notify_subtitle(session) + end + + def config_for_spec : Smith::Config + @config + end +end + +describe "the notifications a CLI run is wired to send" do + it "resolves to nothing to deliver to in a shell that is not cmux" do + # The default has to be "a plain terminal run is unchanged": nothing is + # delivered, and nothing is even looked for. Resolved against an + # environment passed in rather than the ambient one — these specs run + # inside a real cmux terminal more often than not, and a test that reads + # `ENV` would then assert the opposite of what it claims. + temp_dir = File.join(Dir.tempdir, "smith_wire_#{Random::Secure.hex(4)}") + previous = ENV["SMITH_HOME"]? + ENV["SMITH_HOME"] = temp_dir + + begin + cli = Smith::CLI.new([] of String) + + resolved = Smith::CmuxClient.resolve(cli.config_for_spec.notify, {} of String => String?) + resolved.enabled?.should be_false + resolved.deliverable?.should be_false + ensure + previous ? (ENV["SMITH_HOME"] = previous) : ENV.delete("SMITH_HOME") + FileUtils.rm_rf(temp_dir) + end + end + + it "delivers nothing yet, inside cmux or not" do + # The seam this stops at, asserted rather than assumed: `Notify#enabled?` + # reports whether there is a client that can deliver, and until the socket + # is spoken to there is only the null one. So the wiring is complete and + # still sends nothing — which is why `build_agent` can attach it + # unconditionally instead of branching on "am I inside cmux?". + temp_dir = File.join(Dir.tempdir, "smith_wire_#{Random::Secure.hex(4)}") + previous = ENV["SMITH_HOME"]? + ENV["SMITH_HOME"] = temp_dir + + begin + cli = Smith::CLI.new([] of String) + notify = cli.notify_for_spec + + notify.enabled?.should be_false + # …and a run finishing still costs nothing and still fails nothing. + notifier = Smith::TurnNotifier.new(notify) + notifier.handle(Smith::Events::TurnCompleted.new(1)) + ensure + previous ? (ENV["SMITH_HOME"] = previous) : ENV.delete("SMITH_HOME") + FileUtils.rm_rf(temp_dir) + end + end + + it "names the project a session was started in" do + # Read off the session rather than `Dir.current`: a resumed session runs + # wherever it was created, and that is the name worth reading from another + # tab. Built rather than created, because `Store#create` writes a session + # file — and a spec has no business leaving one in the developer's + # `~/.smith`. + session = Smith::Session::Data.new(id: "spec-session", cwd: "/work/smith", model: "mock-model", provider: "mock") + + Smith::CLI.new([] of String).notify_subtitle_for_spec(session).should eq("smith") + end + + it "prefers a session name when it has one" do + session = Smith::Session::Data.new( + id: "spec-session", + cwd: "/work/smith", + model: "mock-model", + provider: "mock", + name: "notify-work" + ) + + Smith::CLI.new([] of String).notify_subtitle_for_spec(session).should eq("smith · notify-work") + end + + it "still names something when there is no session yet" do + # A headless run has one, but the helper must not depend on it: it is + # called from the same place the agent is built, and a nil session is a + # state that reaches it. + Smith::CLI.new([] of String).notify_subtitle_for_spec(nil).should eq(File.basename(Dir.current)) + end +end diff --git a/spec/smith/config_spec.cr b/spec/smith/config_spec.cr index cbb9e45..32ae8e4 100644 --- a/spec/smith/config_spec.cr +++ b/spec/smith/config_spec.cr @@ -839,3 +839,102 @@ describe "session settings" do end end end + +# Only the config file tier: merging the `CMUX_*` environment into this is +# `Smith::CmuxClient.resolve`, covered in cmux_notify_spec.cr. Keeping the two +# apart here is what makes it possible to say which tier a value came from. +describe "notify settings" do + it "is off and empty until a [notify] section says otherwise" do + with_sandbox do |temp_dir, _home| + settings = Smith::Config.load(make_project(temp_dir)).notify + + # nil, not false: nobody asked either way, and that is the answer + # `CmuxClient.resolve` lets the terminal overrule. + settings.enabled.should be_nil + settings.socket_path.should be_nil + settings.surface_id.should be_nil + settings.workspace_id.should be_nil + settings.timeout.should eq(1.0) + end + end + + it "reads the section" do + with_sandbox do |temp_dir, _home| + project = make_project(temp_dir, <<-TOML) + [notify] + enabled = true + socket_path = "/tmp/cmux.sock" + surface_id = "surface:1" + workspace_id = "workspace:1" + timeout = 2.5 + TOML + + settings = Smith::Config.load(project).notify + + settings.enabled.should be_true + settings.socket_path.should eq("/tmp/cmux.sock") + settings.surface_id.should eq("surface:1") + settings.workspace_id.should eq("workspace:1") + settings.timeout.should eq(2.5) + settings.socket?.should be_true + settings.deliverable?.should be_true + end + end + + it "lets the project config override the global one, key by key" do + with_sandbox do |temp_dir, home_dir| + File.write(File.join(home_dir, "config.toml"), <<-TOML) + [notify] + enabled = true + socket_path = "/global/cmux.sock" + surface_id = "global-surface" + TOML + + project = make_project(temp_dir, <<-TOML) + [notify] + surface_id = "project-surface" + TOML + + settings = Smith::Config.load(project).notify + + settings.enabled.should be_true + settings.socket_path.should eq("/global/cmux.sock") + settings.surface_id.should eq("project-surface") + end + end + + it "turns a blank setting into unset rather than into an empty value" do + # An empty `socket_path` is a key someone left behind, not a location. Read + # as a location it would shadow the environment's with nothing. + with_sandbox do |temp_dir, _home| + project = make_project(temp_dir, <<-TOML) + [notify] + socket_path = " " + surface_id = "" + TOML + + settings = Smith::Config.load(project).notify + + settings.socket_path.should be_nil + settings.surface_id.should be_nil + settings.socket?.should be_false + end + end + + it "ignores values of the wrong type instead of refusing to start" do + with_sandbox do |temp_dir, _home| + project = make_project(temp_dir, <<-TOML) + [notify] + enabled = "yes" + socket_path = 42 + timeout = "soon" + TOML + + settings = Smith::Config.load(project).notify + + settings.enabled.should be_nil + settings.socket_path.should be_nil + settings.timeout.should eq(1.0) + end + end +end diff --git a/src/smith/cli.cr b/src/smith/cli.cr index 23a1115..9bda220 100644 --- a/src/smith/cli.cr +++ b/src/smith/cli.cr @@ -27,6 +27,7 @@ require "./update" require "./doctor" require "./marketplace" require "./ui" +require "./turn_notifier" module Smith class CLI @@ -73,6 +74,7 @@ module Smith # keep the plain renderer even on a TTY. @interactive_tui : Bool = false @tui_app : UI::App? = nil + @notify : Notify? = nil @tui_warned : Bool = false @update_check : Bool = false @allow_unverified : Bool = false @@ -684,6 +686,26 @@ module Smith end end + # Where completion notifications go. Built once, and a no-op anywhere cmux + # is not the terminal in use — so no call site has to ask first. Which + # config and which environment decide that is `Notify`'s business, and the + # reason this names one class rather than two. + private def notify : Notify + @notify ||= Notify.build(@config.notify) + end + + # Which session this is, in one glance from another tab. The project + # directory is the part that tells two smith runs apart, since they are + # usually the same project run twice rather than two projects; a session + # that was named says its name, because that is what the user would have + # called it. + private def notify_subtitle(session_data : Session::Data?) : String? + project = File.basename(session_data.try(&.cwd) || Dir.current) + name = session_data.try(&.name) + + name.nil? || name.empty? ? project : "#{project} · #{name}" + end + # Takes the whole session rather than its pieces: passing messages and the # calibration ratio separately is how one call site came to carry the # transcript without what had been learned about measuring it. @@ -817,6 +839,16 @@ module Smith renderer.handle(event) end + # A second listener rather than a branch inside the first: a + # notification is not a rendering choice, and every renderer — plain, + # JSON, fullscreen — is served by the same one. Fresh per agent, so the + # subtitle is the session this agent runs and the text collected for it + # cannot carry over into one that replaced it. + notifier = TurnNotifier.new(notify, subtitle: notify_subtitle(session_data)) + agent.on_event do |event| + notifier.handle(event) + end + plan = plan_session plan.on_plan = ->(text : String) do renderer.handle(Events::PlanPresented.new(text)) diff --git a/src/smith/cmux_client.cr b/src/smith/cmux_client.cr new file mode 100644 index 0000000..c37788a --- /dev/null +++ b/src/smith/cmux_client.cr @@ -0,0 +1,171 @@ +require "./cmux_clientable" +require "./null_cmux_client" +require "./notify_config" + +module Smith + # The environment half of a cmux notification setup. `NotifyConfig.from_table` + # reads what the config file said; this merges in the `CMUX_*` variables the + # terminal exports into every process it spawns, decides which of the two + # wins, and turns the result into something `Smith::Notify` can talk to. + # + # This is the only place in smith that knows those variable names, that a + # socket is involved, or that a protocol exists. `Smith::Notify` sees a + # resolved config and a `CmuxClientable`; neither knows there is an + # environment, and neither reaches for one. + module CmuxClient + # In priority order. `CMUX_SOCKET_PATH` is the one cmux documents and the + # one its environment actually carries. + # + # The other two are kept because they cost nothing to check and because a + # spelling smith refused to read is a notification that silently does not + # arrive: `CMUX_SOCKET` is exported alongside the documented name — empty, + # in the environment this was written against, which is exactly why a + # resolution has to read the first value that says something rather than + # the first variable that is set — and `CMUX` is the spelling #120 named + # and the one a wrapper script is most likely to set itself. + SOCKET_PATH_KEYS = {"CMUX_SOCKET_PATH", "CMUX_SOCKET", "CMUX"} + + # Two pairs of names for the same two values. cmux's own CLI and docs speak + # of tabs and panels, and the environment this was written against exports + # `CMUX_WORKSPACE_ID` and `CMUX_SURFACE_ID` carrying the same two ids its + # `CMUX_TAB_ID` and `CMUX_PANEL_ID` do — both halves verified, not assumed. + # + # Reading both costs nothing, and which pair a build exports is not + # something smith can ask about. The workspace and surface names come + # first: they are the ones observed, and the tab and panel names are the + # ones a build might stop exporting. + WORKSPACE_ID_KEYS = {"CMUX_WORKSPACE_ID", "CMUX_TAB_ID"} + SURFACE_ID_KEYS = {"CMUX_SURFACE_ID", "CMUX_PANEL_ID"} + + # Values an environment variable can hold that mean "not set" rather than + # "set to this": a shell that exports one of these is saying the same thing + # as one that never exported it, so the next tier down gets its say. + # + # Not a hypothetical. cmux exports `CMUX_SOCKET=` empty alongside a + # populated `CMUX_SOCKET_PATH` in the same environment, so a resolution + # that read the first set *variable* rather than the first set *value* + # would find no socket at all — and would then conclude the session is not + # running inside cmux, because the socket is what says so. + FALSEY = {"", "0", "false", "no", "off"} + + # Config plus environment, environment winning. Inside cmux the variables + # describe the terminal that is running right now — this surface, this + # workspace, this socket — so they are the more accurate answer than + # anything a config file could have been written with. + # + # `env` is a parameter rather than `ENV` so the resolution is testable + # without touching the process environment. + def self.resolve(config : NotifyConfig, env : Hash(String, String?) = env_snapshot) : NotifyConfig + # A socket cmux itself put into the environment, as opposed to one a + # config file named. Kept apart because the two mean different things: + # the first says "this terminal is inside cmux, right now", the second + # only says "here is a location". + live_socket = socket_from_env(env) + + NotifyConfig.new( + enabled: enabled(config, live_socket), + socket_path: live_socket || config.socket_path, + surface_id: first_of(env, SURFACE_ID_KEYS) || config.surface_id, + workspace_id: first_of(env, WORKSPACE_ID_KEYS) || config.workspace_id, + timeout: config.timeout + ) + end + + # Resolve, then build. The one call a caller that is not itself resolving + # anything needs. + def self.build(config : NotifyConfig, env : Hash(String, String?) = env_snapshot) : CmuxClientable + client(resolve(config, env)) + end + + # The client for an already-resolved config. Kept separate from `resolve` + # because resolving is pure: a diagnostic can compute the effective config + # without anything being connected. + # + # Null whenever delivering is impossible, so the caller never has to ask + # first. + def self.client(config : NotifyConfig) : CmuxClientable + return NullCmuxClient.new unless config.deliverable? + + # TODO: open the unix socket at `config.socket_path` and speak to it + # (#120). Until then every config resolves to null — the plumbing is + # here, the wire is not, and nothing silently claims a notification was + # delivered. + NullCmuxClient.new + end + + # Whether notifications go out. Three answers, because there are three + # questions and only two of them belong to the config file: + # + # An explicit `enabled = false` is honoured no matter what the terminal + # says — that is somebody turning them off, and being inside cmux is not a + # reason to overrule them. An explicit `true` is honoured as readily. + # + # Absent is nobody having said, and that is where the terminal gets its + # say: cmux announcing a socket *is* the announcement that this session is + # running inside it, which is the situation a completion notification + # exists for. So the default is on inside cmux and off everywhere else, + # and a run started in a plain terminal is unchanged by this feature. + # + # Deliberately not decided by whether a socket path resolved *from config*: + # pointing at a location is not the same statement as "you are inside me", + # and treating it as one would switch notifications on for a config file + # that only ever meant to say where the socket is. + private def self.enabled(config : NotifyConfig, live_socket : String?) : Bool + explicit = config.enabled + return explicit unless explicit.nil? + + !live_socket.nil? + end + + private def self.socket_from_env(env : Hash(String, String?)) : String? + SOCKET_PATH_KEYS.each do |key| + value = truthy(env, key) + next if value.nil? + # A bare `CMUX` holds a flag in the wild — `CMUX=1` — and reading that + # as a location would connect to a file called `1` in the current + # directory. Only something shaped like a path is taken as one. + next if key == "CMUX" && !value.includes?("/") + return value + end + + nil + end + + # The first of `keys` that holds a value saying something, so a documented + # spelling can stand in for an exported one without either being assumed. + private def self.first_of(env : Hash(String, String?), keys : Enumerable(String)) : String? + keys.each do |key| + value = presence(env, key) + return value unless value.nil? + end + + nil + end + + # A set variable that says something. Whitespace-only and the usual + # spellings of "off" come back as nil, which is what lets the tier below + # speak. + private def self.truthy(env : Hash(String, String?), key : String) : String? + value = NotifyConfig.normalize(env[key]?) + return nil if value.nil? + FALSEY.includes?(value.downcase) ? nil : value + end + + private def self.presence(env : Hash(String, String?), key : String) : String? + NotifyConfig.normalize(env[key]?) + end + + # A copy of the process environment, in the type the resolution works in. + # + # Not `ENV` itself as the default: `ENV` is not a `Hash`, and a default + # argument is only checked where the method is actually called — so + # `= ENV` sat unobjected until something called `resolve`, and it broke the + # build rather than failing quietly. Snapshotting also means a resolution + # cannot observe the environment changing underneath it mid-call. + private def self.env_snapshot : Hash(String, String?) + snapshot = Hash(String, String?).new + ENV.each { |key, value| snapshot[key] = value } + snapshot + end + end +end diff --git a/src/smith/cmux_clientable.cr b/src/smith/cmux_clientable.cr new file mode 100644 index 0000000..7c98c58 --- /dev/null +++ b/src/smith/cmux_clientable.cr @@ -0,0 +1,25 @@ +require "json" + +module Smith + # The seam `Smith::Notify` talks to: something that can hand a cmux + # notification payload to a cmux daemon. + # + # Deliberately the whole surface. It exposes no socket, no path and no + # protocol, so neither the caller nor the specs need to know which one the + # real implementation happens to speak — and the null one can keep pretending + # there is a daemon at all. + abstract class CmuxClientable + # Hand a ready-made notification payload to cmux. + # + # `payload` is what goes over the wire, keys and all — implementations only + # transport it. Returns true when cmux accepted it, false when it did not + # or when there was nothing to accept. A failure here is never an + # exception: notification delivery must not be able to take a run down. + abstract def notify(payload : Hash(String, JSON::Any)) : Bool + + # True when there is somewhere to deliver to. Used for diagnostics, never + # as a precondition — `notify` on a client that answers false is a no-op, + # not an error. + abstract def available? : Bool + end +end diff --git a/src/smith/config.cr b/src/smith/config.cr index 825e4a4..aab277a 100644 --- a/src/smith/config.cr +++ b/src/smith/config.cr @@ -9,6 +9,7 @@ require "./mentions" require "./sandbox" require "./media" require "./pricing" +require "./notify_config" module Smith # Resolved configuration, merged from (lowest to highest priority): @@ -627,6 +628,18 @@ module Smith ) end + # The `[notify]` section, and only that: what the config file says about + # cmux desktop notifications. + # + # The `CMUX_*` environment the terminal exports is the other half, and this + # deliberately does not reach for it — a config file and a terminal are two + # questions, and keeping them apart is what lets either be reasoned about. + # The record does the reading: it is pure data, so no name that knows about + # sockets or variables appears here at all. + def notify : NotifyConfig + NotifyConfig.from_table(lookup("notify").try(&.as_h?)) + end + # Consumed by Subagents::Supervisor via CLI#build_agent. max_children = 0 # switches subagents off entirely — the agent tool is then not registered. def subagents : SubagentSettings diff --git a/src/smith/notify.cr b/src/smith/notify.cr new file mode 100644 index 0000000..9b5f768 --- /dev/null +++ b/src/smith/notify.cr @@ -0,0 +1,131 @@ +require "json" +require "./cmux_client" +require "./cmux_clientable" +require "./null_cmux_client" +require "./notify_config" + +module Smith + # Decides that a notification should go out, and what it says. Nothing + # more: which socket carries it, which verb cmux wants and whether the + # daemon is even running are the client's business, and this class never + # asks. + # + # It is built once per run from a `CmuxClientable` — the real one when cmux + # is reachable, `NullCmuxClient` in every other case — so no caller has to + # branch on "am I inside cmux?". The same line of code works either way and + # just does nothing when there is nowhere to deliver to. + class Notify + # What a payload field may hold. Recursive so extras can carry a small + # nested value, and narrow enough that a caller passing something + # unserialisable finds out at compile time rather than at the end of a + # long run. + alias Field = String | Int32 | Int64 | Float64 | Bool | Nil | Array(Field) | Hash(String, Field) + + # The discriminant of what kind of message this is. cmux also accepts + # status lines and progress updates; naming the kind here is what lets one + # client speak to all of them later without this class growing a second + # method per message type. + TYPE = "notification" + + def initialize(@client : CmuxClientable) + end + + # The one door the rest of smith goes through: hand over the resolved + # config, get back something that notifies or silently does not. No caller + # has to ask which client a config deserves, whether a socket is involved + # or whether cmux is the terminal in use — and so no caller learns the name + # of anything that knows, which is what keeps "the rest of smith knows + # none of these three" true rather than approximately true. + # + # The resolution lives in `CmuxClient`, not here: this class still knows no + # environment, no socket and no protocol, and only names the thing that + # does. A constructor taking a client stays, because that is how a spec + # hands over a recording one. + def self.build(config : NotifyConfig) : Notify + new(CmuxClient.build(config)) + end + + # True when there is somewhere to deliver to. Purely informational — the + # caller does not need it to call `notify`, and should not skip on it: a + # no-op is exactly what "not inside cmux" means here. + def enabled? : Bool + @client.available? + end + + # Send a notification. Returns true only when cmux took it; false covers + # "nowhere to send", "cmux refused" and "something threw", which the + # caller has no way to tell apart and no reason to. + # + # `title` is always sent, even empty — it is the one field a notification + # is identified by. `subtitle` and `body` are sent only when they say + # something, because an empty body renders as a gap rather than as nothing. + # + # `extra` lands in the payload as given and wins over `type`, `subtitle` + # and `body`: a caller that wants a different discriminant, or wants a + # blank body kept, says so here instead of rebuilding the payload. `title` + # cannot be overridden this way — it is a declared parameter, and Crystal + # refuses a named argument that repeats one. + # + # The splat is untyped and the `Field` restriction is enforced by + # `to_any` instead. Restricting it here would be the more honest + # signature, but a typed double splat on a method that also has defaulted + # arguments cannot be called without naming one — Crystal 1.21 rejects + # `notify("x")` — and "notify with nothing but a title" is the common case. + def notify(title : String, subtitle : String? = nil, body : String? = nil, **extra) : Bool + payload = Hash(String, JSON::Any).new + payload["type"] = JSON::Any.new(TYPE) + payload["title"] = JSON::Any.new(title) + put(payload, "subtitle", subtitle) + put(payload, "body", body) + + extra.each do |key, value| + payload[key.to_s] = to_any(value) + end + + deliver(payload) + end + + # A client that is not supposed to throw says so in its contract, and a + # real one that talks to a socket will anyway — the socket can vanish + # between the check and the write, and cmux can be killed mid-run. Either + # way this is the last thing a finishing run does, and "the notification + # failed" must never be the reason a run fails. + private def deliver(payload : Hash(String, JSON::Any)) : Bool + @client.notify(payload) + rescue ex : Exception + false + end + + private def put(payload : Hash(String, JSON::Any), key : String, value : String?) : Nil + return if value.nil? + stripped = value.strip + return if stripped.empty? + + payload[key] = JSON::Any.new(stripped) + end + + # `Field` down to something `JSON::Any` can hold. Written out rather than + # round-tripped through `to_json` and `JSON.parse` because the recursion + # has to happen here: an `Array(Field)` or a `Hash(String, Field)` is not + # itself a JSON type, only its leaves are, and a leaf arrives as an `Int32` + # where `JSON::Any` wants an `Int64`. A value the alias does not admit is + # refused where it is passed, which is the point of having the alias. + private def to_any(value : Field) : JSON::Any + case value + when Array + JSON::Any.new(value.map { |item| to_any(item) }) + when Hash + JSON::Any.new(value.to_h { |key, item| {key, to_any(item)} }) + when Int32 + JSON::Any.new(value.to_i64) + else + # Nil, Bool, Int64, Float64 and String: the scalar half of + # `JSON::Any::Type`, so they go over as they are. This `else` is a + # branch rather than the fifth `when` because Crystal cannot prove a + # recursive alias exhaustive, and an unreachable `raise` would be the + # less honest of the two ways to say so. + JSON::Any.new(value) + end + end + end +end diff --git a/src/smith/notify_config.cr b/src/smith/notify_config.cr new file mode 100644 index 0000000..0e7a18b --- /dev/null +++ b/src/smith/notify_config.cr @@ -0,0 +1,86 @@ +require "toml" + +module Smith + # Resolved configuration for cmux desktop notifications. Pure data: it holds + # the effective values after config and environment have been merged, and + # knows nothing about sockets or the wire protocol. + # + # Reading the config tier is here for the same reason — a TOML table is data, + # and turning it into this record involves no environment, no socket and no + # protocol. Which keeps the callers of the notify subsystem down to one name: + # `Config` reads a table into this record, and everything downstream hands the + # record to `Notify`. The environment is somebody else's business. + # + # Blank strings are normalised to `nil` so callers can treat "unset" and + # "explicitly empty" the same way. + # + # `enabled` is a `Bool?` for the same reason, and it matters more here than + # anywhere else in this record: `false` is somebody turning notifications + # off, `nil` is nobody having said. Only the second one lets the terminal + # have a say. Collapsing the two would mean a config file that never mentions + # `[notify]` was read as one that refused it, and no amount of environment + # could switch them back on. + record NotifyConfig, + enabled : Bool? = nil, + socket_path : String? = nil, + surface_id : String? = nil, + workspace_id : String? = nil, + timeout : Float64 = DEFAULT_TIMEOUT do + DEFAULT_TIMEOUT = 1.0 + + # The `[notify]` section, before the environment has had a say. + # + # Blank strings become nil, so `socket_path = ""` in a config file is the + # same as the key not being there — otherwise the empty value would shadow + # the environment with nothing. A value of the wrong type is ignored rather + # than raised on, because `socket_path = true` is a typo and not a reason + # for smith to refuse to start. + def self.from_table(table : Hash(String, TOML::Any)? = nil) : NotifyConfig + timeout = float_setting(table, "timeout") + + NotifyConfig.new( + enabled: setting(table, "enabled").try(&.as_bool?), + socket_path: normalize(setting(table, "socket_path").try(&.as_s?)), + surface_id: normalize(setting(table, "surface_id").try(&.as_s?)), + workspace_id: normalize(setting(table, "workspace_id").try(&.as_s?)), + timeout: timeout.nil? || timeout <= 0 ? DEFAULT_TIMEOUT : timeout + ) + end + + # One rule for both tiers, so "unset" and "explicitly empty" cannot come + # apart between the config file and the environment: a whitespace-only + # value is an absent one, wherever it was read from. + def self.normalize(value : String?) : String? + return nil if value.nil? + stripped = value.strip + stripped.empty? ? nil : stripped + end + + # True when a socket path was resolved. Without one there is no cmux + # daemon to talk to, so notifications degrade to a no-op. + def socket? : Bool + !@socket_path.nil? + end + + # True when notifications were asked for. `nil` is not: nobody asked, and + # this record on its own has nothing to go on. + def enabled? : Bool + @enabled == true + end + + # True when everything needed to actually deliver is present. + def deliverable? : Bool + enabled? && socket? + end + + private def self.setting(table : Hash(String, TOML::Any)?, key : String) : TOML::Any? + table.try(&.[key]?) + end + + # TOML writes `timeout = 2` as an integer and `timeout = 0.5` as a float, + # and both are the same statement — `as_f?` reads either. + private def self.float_setting(table : Hash(String, TOML::Any)?, key : String) : Float64? + setting(table, key).try(&.as_f?) + end + end +end diff --git a/src/smith/null_cmux_client.cr b/src/smith/null_cmux_client.cr new file mode 100644 index 0000000..a305573 --- /dev/null +++ b/src/smith/null_cmux_client.cr @@ -0,0 +1,20 @@ +require "./cmux_clientable" + +module Smith + # The client for every case where there is nothing to deliver to: cmux + # notifications are off, no socket was resolved, or the daemon is not there. + # + # It exists so the caller does not branch. `Smith::Notify` builds a client + # and calls `notify` either way, and the difference between "cmux is not + # running" and "cmux is running" never reaches the code that decides a run + # is finished and the human should hear about it. + class NullCmuxClient < CmuxClientable + def notify(payload : Hash(String, JSON::Any)) : Bool + false + end + + def available? : Bool + false + end + end +end diff --git a/src/smith/turn_notifier.cr b/src/smith/turn_notifier.cr new file mode 100644 index 0000000..8097c21 --- /dev/null +++ b/src/smith/turn_notifier.cr @@ -0,0 +1,104 @@ +require "./events" +require "./notify" + +module Smith + # The first consumer of `Smith::Notify`: it listens to a run and turns "this + # turn is over" into one notification. + # + # A listener of its own rather than a fourth renderer, because which + # renderer is showing the run is irrelevant to whether one should go out — a + # `--json` caller and a fullscreen one are equally served by being told when + # to look back — and because a renderer owns the exit code and the failure + # state of the run, which this has no business deciding. + # + # Attached in `CLI#build_agent`, the one place every route to a main-thread + # agent passes through, so the plain loop, the fullscreen one, `run`, + # `resume` and a mid-session `/resume` all notify without any of them having + # to know. + # + # A subagent is deliberately left out, and not by a check for it: children + # are built by `Subagents::Supervisor` with `Agent.new` of its own and given + # their own listener, so they never meet this one. That is also the right + # answer — a delegate that announced each child as it finished would be + # noise where the parent's own completion is the signal. cmux agrees, to the + # extent that it suppresses subagent completions of its own agents; note that + # its `CMUX_SUPPRESS_SUBAGENT_NOTIFICATIONS` governs events cmux derives from + # the wrappers it installs for Claude Code and Codex, not anything posted + # over the socket, so smith cannot lean on it here. + # + # It fires on every turn rather than only on long ones, and deliberately does + # not try to guess whether anybody is watching: cmux already withdraws the + # banner of a workspace that has become visible, so the decision belongs to + # the terminal that knows its own focus — which is the one thing this cannot + # see from inside. + class TurnNotifier + # A body is read at a glance, from another tab, while deciding whether to + # switch back. What does not fit that is not a body: a model's closing + # answer runs to pages, and a notification carrying three of them is one + # nobody reads. + # + # Smith's own limit — cmux documents a title, a subtitle and a body + # without a length on any of them, so nothing here is imposed by the + # terminal. Cut at a word boundary and marked as cut, rather than sent + # whole or dropped: the reader should get the sentence the run ended on + # and be told there is more. + MAX_BODY = 200 + + def initialize(@notify : Notify, @subtitle : String? = nil) + @text = "" + end + + def handle(event : Events::Event) : Nil + case event + when Events::AssistantText + # Collected rather than sent: one response can carry several text + # blocks, and only what the last one said is the answer. + @text += event.text + when Events::ToolStart + # Text before a tool call was an announcement — "let me look at that + # file" — not an answer. Dropped, so a run that ends among its tools + # reports no body rather than a stale promise of one. + @text = "" + when Events::TurnCompleted + announce + when Events::TurnError, Events::BudgetExceeded, Events::ContextExhausted + # A run can end on any of these instead of a completed turn — a + # provider that failed, a budget that ran out, a window that filled. + # Whatever text was collected belongs to the run that just died, and + # left standing it would be prefixed to the next turn's answer. + @text = "" + end + end + + # The turn is over. Title names who, subtitle names where, and the body + # says what came out — the three fields `cmux notify` documents, and + # nothing beyond them: what else a payload may carry is a question for the + # wire, which does not exist yet (#120). + private def announce : Nil + begin + @notify.notify("Smith", subtitle: @subtitle, body: body) + ensure + # The text belonged to the turn that just ended. Left standing it would + # be prefixed to the next one's answer, and a session of four turns + # would notify four times with the fourth body holding all four. + @text = "" + end + end + + # One line of prose, cut at a word boundary. Whitespace is collapsed + # because a model's answer usually starts with a paragraph, and a body + # carrying twelve newlines reads as a gap rather than as a message. + private def body : String? + collapsed = @text.gsub(/\s+/, " ").strip + return nil if collapsed.empty? + return collapsed if collapsed.size <= MAX_BODY + + cut = collapsed[0, MAX_BODY] + if (space = cut.rindex(' ')) && space > MAX_BODY // 2 + cut = cut[0, space] + end + + "#{cut}…" + end + end +end