feat(acp): post owner control command outcomes back into the channel - #3916
feat(acp): post owner control command outcomes back into the channel#3916Berenger-Wooclap wants to merge 3 commits into
Conversation
|
nice closing the loop on !shutdown/!cancel/!rotate — please keep the channel notice short and avoid dumping stack traces; owners need the outcome, not the harness internals. |
`!shutdown`, `!cancel` and `!rotate` are consumed by the harness and never reach the agent, so the only trace they leave is a `tracing::info!` line in the harness log. From the chat a successful rotate, a no-op cancel, and an event the harness never received all look identical: your own message, then silence. Post the outcome as a kind:9 notice into the channel the command came from, reusing the best-effort helper the dead-letter path already uses. The no-op cases are the ones that matter most — they are invisible today and read as a broken command. `!shutdown` awaits its notice instead of spawning it: the harness is about to exit and a spawned task would race the shutdown. Renames `post_failure_notice` to `post_notice` — it is no longer failure-only. Refs block#3711 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Bérenger Ouadi <berenger.ouadi@wooclap.com>
c82427b to
cc911a9
Compare
Chessing234
left a comment
There was a problem hiding this comment.
posting control-command outcomes as kind:9 notices closes a real silent-failure gap (especially !cancel no-op). awaiting the notice before shutdown is the important detail — a spawned task would race exit. worth confirming the notice inherits the same thread tags as the command so it doesn't land as a root channel message when the owner typed in a thread.
|
Thanks — agreed, and that constraint is already load-bearing here, so let me make it explicit. All six notices are fixed one-line sentences, and the types enforce it:
The One thing worth separating, since the rename in this PR may be what prompted the caution: the dead-letter notice that now shares One design question while you're hereThis PR makes the agent say these lines: the notice is a kind:9 signed with the agent's own key, so it renders as an ordinary message from the agent. But a cancelled turn or a rotated session is arguably a lifecycle event rather than the agent speaking, and the alternative I listed in #3711 is a kind:40099 system row instead. I looked into what that would actually cost, in case it helps you decide:
So: agent as it stands, or a system row? I lean slightly toward the system row on aesthetics. |
Review asked to confirm that the notice inherits the thread tags of the command, so a `!rotate` typed in a thread is not answered with an unrelated root-level message. It does — `spawn_control_notice` and the awaited `!shutdown` path both derive their `ThreadTags` from the triggering event — but nothing guarded it. Extract the `ThreadTags` to `ThreadRef` mapping out of `post_notice` into `thread_ref_from_tags` so the test exercises the real path rather than a copy of it, and assert both directions: a threaded command keeps its root and parent, a channel-level command stays unthreaded. Verified the test fails when `thread_ref_from_tags` is made to return `None`. Refs block#3711 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Bérenger Ouadi <berenger.ouadi@wooclap.com>
Confirmed. I made a small comit to test this. |
Switches the acknowledgement from a kind:9 message to a kind:40099 system
message. A cancelled turn or a rotated session is a lifecycle event, not the
agent speaking, so it belongs in the grey system row beside "X joined the
channel" rather than in the conversation.
The row still lands in the thread the command was typed in: client thread
placement is derived from NIP-10 `e` tags and is kind-agnostic, so the system
message carries the same tags a kind:9 notice did.
Payload is `{type, actor, target}` — `actor` the owner who issued the command,
`target` the agent it acted on — matching every other system message, so both
renderers resolve them to profile names and prefetch their profiles. Six types,
one per outcome, including the two no-ops.
Adds `buzz_sdk::build_system_message` so the NIP-10 tag construction is shared
with `build_message` instead of duplicated in the harness, and cases in both
renderers: an unknown `type` is dropped silently by each, so a client older
than this contract shows no row at all. That is the one regression against the
kind:9 shape, which rendered everywhere.
Reverts the `post_failure_notice` rename from the first commit: the kind:9
helper is failure-only again now that control outcomes have their own path.
Refs block#3711
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Bérenger Ouadi <berenger.ouadi@wooclap.com>
|
Answering my own question above: I've switched this to the kind:40099 system row, pushed as 496f3bd. It renders as a system line beside "X joined the channel" rather than as a message from the agent, which is the right weight for a lifecycle event. Screenshot of all six outcomes is in the PR description. Two things I checked that made the switch safe, in case they're useful:
The honest cost, spelled out in the description: an unknown I also owe you a correction: I only just noticed #3877 implements this same slice, opened before mine. Sorry for the duplicated review load. If that one is the shape you want, close this and I'll send the thread-placement test and the README update on top of it instead. |
Implements the acknowledgement half of #3711.
What problem this solves
!shutdown,!canceland!rotateare consumed by the harness and never reach the agent, so the only trace they leave is a line in the harness log. From the chat, a successful rotate, a no-op cancel, and a command the harness never received are indistinguishable: your own message, then silence. For controls whose whole purpose is to steer a session you cannot otherwise observe, that is the one thing that should be visible.How it looks
The outcome is published as a kind:40099 system message, so it renders as a system row — the same grey line as "X joined the channel" — rather than as a message from the agent. A cancelled turn or a rotated session is a lifecycle event, not the agent speaking.
The reading in the channel is a pair: your command, then the row it produced. The command itself stays visible — the harness consumes it so it never reaches the agent, but the event is still stored and rendered.
All six outcomes, seeded through the E2E mock bridge (
just desktop-screenshot --messages):!cancel!cancel!rotate!rotate!rotate!shutdownThe no-op rows matter most: they are the cases that are invisible today and that read as a broken command.
Key decisions
etags and is kind-agnostic, so the system message carries the same tags a kind:9 notice would. A!rotatetyped in a thread is answered in that thread, not at channel root. Pinned by a regression test.!shutdownawaits its notice instead of spawning it — the harness is about to exit and a spawned task would race the shutdown.{type, actor, target},actorbeing the owner who issued the command andtargetthe agent it acted on. Same two fields every other system message uses, so both renderers resolve them to profile names and prefetch their profiles, and the channel keeps a record of who issued the command.buzz_sdk::build_system_messageso the NIP-10 tag construction is shared withbuild_messagerather than duplicated in the harness.The trade-off, stated plainly
An unknown
typeis dropped silently by both renderers (describeSystemEvent→default: return null;SystemEvent.fromContent→_ => null). A client older than this contract shows no row at all, where a kind:9 notice rendered everywhere, including in third-party Nostr clients. That is the cost of the system-row shape, and the reason this started as kind:9. I went with the system row because the alternative puts lifecycle events in the conversation permanently; happy to go back if you weigh the fallback higher.Relationship to #3877
#3877 implements the same slice of #3711 as a kind:9 notice, and was opened first. I did not see it until after this branch existed — my apologies for the duplicated effort. If you prefer that shape, close this one; the parts worth salvaging either way are the thread-placement regression test and the README update, and I am happy to send those on top of #3877 instead.
Verification
I did not run the whole
just ci, which additionally builds the desktop and Tauri bundles; the gates above cover every surface this diff touches.Follow-up deferred
The other half of #3711 — rendering
!commands as commands in the composer, mirroring the ACP/palette in #3537 — is not in this PR.