OVOS-STOP-1 v1 — stop pipeline plugin specification#33
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces OVOS-STOP-1, a complete stop pipeline plugin specification, and integrates it across the architecture by updating session field contracts, pipeline dispatch mechanics, and documentation indices to support the stop cascade mechanism. ChangesStop Pipeline Plugin Specification and Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@stop.md`:
- Around line 265-269: The stop plugin selection logic must explicitly handle
blacklisted intents/skills: update the algorithm in sections §4.1 and §6.3 to
state that when evaluating a stop candidate (a handler with an intent of form
"<target_skill_id>:stop" or pipeline-local "<own_pipeline_id>:global_stop"),
first check session.blacklisted_skills and session.blacklisted_intents; if the
handler.owner_id or the intent string is present in the denylist, treat that
candidate as ineligible and skip to the next candidate, and if no eligible
candidates remain return None (no dispatch) rather than escalating or
dispatching anyway; document this precedence and the fallback (skip -> next
candidate -> return None) so behavior for stop() and global_stop() selection is
deterministic.
- Around line 286-296: The fenced code block showing the session.pipeline array
lacks a language tag which triggers MD040; update the fence to include a
language (use "yaml") for the block containing session.pipeline and its entries
(e.g., "stop_high", "converse", "intent_matcher_high", "stop_medium",
"intent_matcher_medium", "stop_low", "intent_matcher_low") so tooling/linters
recognize the language and the MD040 warning is resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+265
to
+269
| A stop plugin MUST honour `session.blacklisted_skills` and | ||
| `session.blacklisted_intents` (PIPELINE-1 §5.3–§5.4). A handler | ||
| whose `owner_id` appears in `blacklisted_skills` MUST NOT be | ||
| pinged or selected as a stop target. | ||
|
|
There was a problem hiding this comment.
Resolve denylist behavior conflict for stop/global_stop dispatch selection.
The spec says stop plugins MUST honor session.blacklisted_intents, but the algorithm doesn’t define what to do when <target_skill_id>:stop or <own_pipeline_id>:global_stop is blacklisted. Different implementations could either return None, escalate, or still dispatch. Please add explicit precedence/fallback rules in §4.1/§6.3 so behavior is deterministic.
Proposed normative patch
## 6.3 Denylists
A stop plugin MUST honour `session.blacklisted_skills` and
`session.blacklisted_intents` (PIPELINE-1 §5.3–§5.4). A handler
whose `owner_id` appears in `blacklisted_skills` MUST NOT be
pinged or selected as a stop target.
+For candidate selection, the plugin MUST also exclude any target
+whose dispatch identity `<owner_id>:stop` appears in
+`blacklisted_intents`.
+If explicit "stop everything" or cascade fallback would produce
+`<own_pipeline_id>:global_stop` and that identity is blacklisted,
+the plugin MUST return `None` (decline) for that utterance.Also applies to: 319-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stop.md` around lines 265 - 269, The stop plugin selection logic must
explicitly handle blacklisted intents/skills: update the algorithm in sections
§4.1 and §6.3 to state that when evaluating a stop candidate (a handler with an
intent of form "<target_skill_id>:stop" or pipeline-local
"<own_pipeline_id>:global_stop"), first check session.blacklisted_skills and
session.blacklisted_intents; if the handler.owner_id or the intent string is
present in the denylist, treat that candidate as ineligible and skip to the next
candidate, and if no eligible candidates remain return None (no dispatch) rather
than escalating or dispatching anyway; document this precedence and the fallback
(skip -> next candidate -> return None) so behavior for stop() and global_stop()
selection is deterministic.
Comment on lines
+286
to
+296
| ``` | ||
| session.pipeline: [ | ||
| "stop_high", # escape hatch | ||
| "converse", | ||
| "intent_matcher_high", | ||
| "stop_medium", | ||
| "intent_matcher_medium", | ||
| "stop_low", | ||
| "intent_matcher_low" | ||
| ] | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced code block.
The example fence should declare a language to satisfy MD040 and keep docs tooling clean.
Proposed fix
-```
+```yaml
session.pipeline: [
"stop_high", # escape hatch
"converse",
"intent_matcher_high",
"stop_medium",
"intent_matcher_medium",
"stop_low",
"intent_matcher_low"
]</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 286-286: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @stop.md around lines 286 - 296, The fenced code block showing the
session.pipeline array lacks a language tag which triggers MD040; update the
fence to include a language (use "yaml") for the block containing
session.pipeline and its entries (e.g., "stop_high", "converse",
"intent_matcher_high", "stop_medium", "intent_matcher_medium", "stop_low",
"intent_matcher_low") so tooling/linters recognize the language and the MD040
warning is resolved.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
JarbasAl
added a commit
that referenced
this pull request
May 27, 2026
Aligns with the project-wide filename convention (filename matches the lowercase spec identifier), matching the recent rename of the other spec files. Adds a new entry to the README's "Utterance lifecycle and matching" spec table under OVOS-CONVERSE-1 with a link to the in-review PR #33. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New normative spec defining the stop pipeline plugin role, the two reserved intent_names `stop` and `global_stop`, the broadcast ping-pong stoppability discovery, the LIFO cascade across `session.active_handlers`, and the `ovos.stop` universal broadcast namespace. Five-topic bus surface: - `ovos.stop.ping` / `ovos.stop.pong` — single-broadcast feasibility query with shared shared response topic - `<target_skill_id>:stop` — standard PIPELINE-1 dispatch to the LIFO head of positive responders - `<stop_plugin_id>:global_stop` — fallback / explicit "stop everything" dispatch; handler emits `ovos.stop` - `ovos.stop` — universal broadcast every component subscribes to, session-scoped cleanup obligation Skills SHOULD self-prune from `active_handlers` when unstoppable; stop subscribers MUST cease only activity keyed to the inbound `session_id`. Spec body is implementation-agnostic; no real-project names appear outside the spec-ID namespace. Depends on: OVOS-MSG-1, OVOS-PIPELINE-1, OVOS-SESSION-1, OVOS-SESSION-2, OVOS-CONVERSE-1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ternal Only the intent_name `stop` is reserved at PIPELINE-1 §7.3, because skills are dispatched on `<skill_id>:stop` and a competing skill-level match would bypass the §4 cascade. `global_stop` is the stop plugin's escalation self-dispatch and remains namespaced under `<stop_plugin_id>:global_stop` — collision-free regardless of any other skill registering the same name. Drop its reservation from §2 and from the orchestrator conformance MUST. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The stop plugin's match-phase mutation of session.active_handlers is part of the cascade's correctness story: removing the dispatched target via Match.updated_session propagates the post-stop state through the rest of the utterance lifecycle without waiting for TTL pruning or size-cap eviction, keeping downstream reads of the list accurate. Reverses the prior §6.2 prohibition for the dispatch-target case only; CONVERSE-1's other mutation pathways (TTL, size cap, activation, self-pruning §4.4) continue to own the remaining lifecycle. A global_stop Match MUST NOT touch active_handlers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ten to both topics Two correctness clarifications: 1. A global_stop Match MUST empty session.active_handlers entirely via Match.updated_session (§5.1, §6.2). Previously global_stop was specified as not touching the list; on reflection, global stop terminates every handler the session is tracking, so the cleared list is the accurate post-stop state and must propagate through the rest of the utterance lifecycle for the same reason §4.1 propagates the single- target removal. 2. Skills MUST subscribe to BOTH <own_skill_id>:stop and ovos.stop. The two are not alternatives — depending on the cascade outcome a skill receives one or the other, and must be ready for either. Lifted the subscription requirements from SHOULD to MUST and split the skill conformance into MUST (subscriptions, session-scoped cleanup, idempotency) and SHOULD (ping-pong participation, self-pruning). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aligns with the project-wide filename convention (filename matches the lowercase spec identifier), matching the recent rename of the other spec files. Adds a new entry to the README's "Utterance lifecycle and matching" spec table under OVOS-CONVERSE-1 with a link to the in-review PR #33. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STOP-1 was added to the spec index table but missing from the role-based reading-order lists. Adds it alongside CONVERSE-1 / CONTEXT-1 / TRANSFORM-1 in the "Building a pipeline plugin?" and "Building an orchestrator?" lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…strator
Resolves STOP-1's CONVERSE-1 dependency by relocating
active_handlers to PIPELINE-1, with reserved-name suppression
of the stamping push.
PIPELINE-1 §7.1 — new dispatch-time stamping bullet: the
orchestrator MUST push {skill_id} onto the head of
session.active_handlers, evicting prior entries with the same
skill_id. The push is suppressed in two cases:
1. pipeline-plugin self-dispatches (skill_id == pipeline_id),
which record no skill activity;
2. dispatches on reserved intent_names listed in §7.3 —
converse, response, stop — which represent continuation
or termination of an already-active skill's participation,
not fresh activation.
The push is applied AFTER Match.updated_session is committed,
so a plugin that needs to remove the target ahead of the
dispatch (the stop cascade) does so via updated_session and
the suppressed push leaves the removal in effect. This makes
pre-dispatch the single, well-defined mutation boundary — no
consumer-side removal protocol, no race between optimistic
pre-removal and a failed handler.
PIPELINE-1 §7.3 — amend the "stamping fires identically" claim
to except the active_handlers push for reserved-name dispatches.
SESSION-1 field registry — active_handlers owner moved from
CONVERSE-1 §2.1 to PIPELINE-1 §7.1; entry shape generalised to
{skill_id, ...} so future specs can extend without
re-registering.
STOP-1 §4.1 / §5.1 / §6.2 — restore the Match.updated_session
removal of the dispatch target on stop matches; global_stop
wipes active_handlers entirely. §6.2 explains the pre-dispatch
boundary: orchestrator commits updated_session, suppresses the
push (reserved-name dispatch), downstream lifecycle sees the
post-stop list. The earlier consumer-removal rule is dropped.
STOP-1 §6.1 — response_mode clearing made conditional ("if
present"); semantics owned elsewhere (CONVERSE-1 when it lands).
STOP-1 §3.4 self-pruning — no longer references CONVERSE-1's
mutation pathway; communicated via any session-carrying Message.
STOP-1 preamble — drop CONVERSE-1 from the dependency list;
add the PIPELINE-1 §7.1 source-of-truth pointer.
APPENDIX preamble + §1.2 narrative updated by user to include
CONVERSE-1, STOP-1, COMMON-QUERY-1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…f reserved names The §7.1 active_handlers stamping rule no longer special-cases pipeline-plugin self-dispatches. The orchestrator applies the §7.0 polymorphism rule uniformly — it does not distinguish skill from pipeline-plugin dispatches — and the push is suppressed strictly on the reserved-name registry. To preserve the architectural intent (the stop plugin's pipeline_id must not land in active_handlers on global stop), global_stop is now a reserved intent_name alongside stop, converse, and response: PIPELINE-1 §7.3 — global_stop added to the reservations table with the stop-plugin escalation semantics. stop also added (was previously implicit from STOP-1's reservation claim). STOP-1 §2 — restore both reservations, with an explicit note that reserving global_stop is what gates the active_handlers suppression. Drops the prior "global_stop is plugin-internal and not reserved" framing; the architectural cost of the new suppression rule is one reserved name, paid back by a cleaner orchestrator contract. STOP-1 §5 / §5.1 / §9 — rewording for the restored reservation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…utcome
Reverts the prior ordering flip and the global_stop reservation.
The stop plugin landing in active_handlers on its own
global_stop dispatch is not a problem to engineer around — it
is the ordinary behaviour of the recency-ordered dispatch
record, and it composes correctly with subsequent ping rounds
(the stop plugin doesn't subscribe to ovos.stop.ping, so it
silently times out as can_handle:false on the next stop).
PIPELINE-1 §7.1 — restore "stamp applied after updated_session
is committed". The dispatched skill_id always lands at the
head of active_handlers unless the intent_name is reserved.
PIPELINE-1 §7.3 — global_stop is not in the reservation table
(only stop, converse, response).
STOP-1 §2 — single reservation (stop). global_stop framed as
"plugin-internal and not reserved" with the note that the stop
plugin's pipeline_id landing in active_handlers via the normal
stamp is the ordinary behaviour, not a special case.
STOP-1 §5.1 — global_stop's updated_session wipes the inbound
active_handlers (so prior active skills are correctly cleared);
the post-dispatch state is [{skill_id: stop_plugin_id}] via the
normal stamp on top. This is the expected outcome.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e picks by timestamp
Replaces the LIFO-by-position cascade selection with explicit
MRU-by-timestamp. The active_handlers list is a set keyed by
skill_id with eviction, not an insertion-ordered list — its
"recency" is properly determined by the activated_at timestamp
each entry carries, not by where it happens to sit in the
array.
SESSION-1 field registry — entry shape restored to
{skill_id, activated_at, ...}.
PIPELINE-1 §7.1 — orchestrator stamps {skill_id, activated_at:
<orchestrator-stamped Unix timestamp in seconds>} on dispatch.
The list is described as "a recency record keyed by
activated_at" rather than "MRU by list position".
STOP-1 §2 reservation table, §3.2, §4.1 step 4 — cascade
selection rewritten as "the most recently activated (highest
activated_at) positive pong responder" instead of "LIFO head
among positives". Wording consistent across the spec.
APPENDIX §4.8 — same fix in the rationale paragraph.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onale asides, tighten to normative skeleton Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…T, timeout bound, multi-tier global_stop, drop stop.response Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s not affect converse eligibility Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…top drains converse_handlers - Dependencies: active_handlers owned by PIPELINE-1, not SESSION-1 - global_stop updated_session: empty converse_handlers (CONVERSE-1 §2.1) as well as active_handlers - §6.2: explain global_stop rationale for draining converse_handlers - §9 MUST list: move empty-on-no-active into MUST; fix pong-timeout grammar; split SHOULD out - §9 SHOULD: add skip-ping-when-empty and 1s-timeout bullets Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onverse re-entry - §5.2: explicit Match construction block clearing active_handlers, converse_handlers, and response_mode atomically; prose asserting stop plugin owns all three cleanups - §5.2: note that global_stop stamp onto active_handlers is intentional — stop plugin MAY converse for follow-up clarifications - §6.1: collapse to targeted-stop-only obligation; cross-ref §5.2 for global_stop case to eliminate duplication Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gaps - §3.1: drop confusing multi-tier shared-pipeline_id prose; replace with single MUST (exactly one ovos.stop per global stop event per session); fold §3.2 match obligations into §3.1 - §3.2 (was §3.3): tighten vocabulary SHOULD to normative form - §4.1 step 2: drop non-RFC-2119 "RECOMMENDED"; use plain "default" - §4.2: add explicit rule that non-responding handlers are treated as can_handle: false and are ineligible as stop targets; note cascade escalates to global_stop — closes the legacy-skill gap - §4.3: define idempotency — duplicate stop MUST be treated as no-op, not a second stop sequence - §6.1: cut rationale sentence - §6.2: cut rationale sentences; keep normative drain rules - §6.3: add blacklisted_intents scope rule — applies to dispatched intent_name, not to the ping broadcast - §5.2: cut "stop plugin owns cleanup" rationale sentence - §9: remove inconsistent SHOULD (step-1 skip is already a MUST in algorithm); remove redundant multi-tier SHOULD; add non-responding handler MUST to plugin conformance; tighten idempotency wording Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- §4.1 step 4: add tie-breaking rule — equal activated_at selects the latest list position (most recently stamped) - §4.2 ping: note session_id carried in Message context per MSG-1 - §4.2 pong: MUST use reply derivation so routing metadata is preserved for remote/client-side skills; source/destination are layer-2 only - §4.3: note that updated_session is not rolled back on handler .error - §5.3: reference MSG-1 for session_id on ovos.stop broadcast - §6.1: add "if no matching entry, field is left unchanged" - §6.3: clarify blacklisted_intents applies to the resolved intent_name; a stop utterance escalated to global_stop is gated by the global_stop entry, not the stop entry - §9 Skill SHOULD: tighten pong wording to match reply-derivation MUST Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JarbasAl
force-pushed
the
spec/stop-1-draft
branch
from
49b70f3 to
0a96972
Compare
§7.3 previously cited '§7.1 routing' but routing (subscription discipline) is §7.2; §7.1 covers context stamping and the active_handlers push. Expand to list all three affected subsections explicitly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JarbasAl
added a commit
that referenced
this pull request
May 27, 2026
…TOP-1 - README: remove 'in review' PR links for CONVERSE-1 (#25) and STOP-1 (#33); both now merged; add STOP-1 to reading-order bullets - appendix/divergences: replace stale 'response-mode counter' reference with current session.response_mode field name Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JarbasAl
added a commit
that referenced
this pull request
May 27, 2026
…TOP-1 - README: remove 'in review' PR links for CONVERSE-1 (#25) and STOP-1 (#33); both now merged; add STOP-1 to reading-order bullets - appendix/divergences: replace stale 'response-mode counter' reference with current session.response_mode field name Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New normative spec OVOS-STOP-1 v1 defining the stop pipeline plugin role: the two reserved intent_names
stopandglobal_stop, the ping-pong stoppability discovery, the LIFO cascade acrosssession.active_handlers, and theovos.stopuniversal broadcast namespace.Design highlights
stop(skill-directed cascade) andglobal_stop(universal broadcast).session.active_handlers: target = most-recent positive pong responder.Match.skill_idcarries the target; dispatch is a standard<target>:stopPIPELINE-1 dispatch with full handler-lifecycle trio.ovos.stop.ping/ovos.stop.pong— no dynamic per-skill topic enumeration. Responders include theirskill_idin the pong payload.global_stoptriggersovos.stopbroadcast — fired on explicit "stop everything" vocab, emptyactive_handlers, or no positive pong responders.active_handlerswhen unstoppable, so future ping rounds bypass them.ovos.stopbroadcast listeners) MUST cease only the activity keyed to the inboundsession_id.ovos.stop.*+ 2 dispatch shapes.Companion specs
forward/reply/response.session.active_handlersrecency list,session.response_mode(cleared viaMatch.updated_sessionon stop).Notes
stop.md) againstorigin/devper the standing one-file-per-PR rule.Test plan
ovos.stop.*namespace reservation does not clash with any existing or in-flight spec.active_handlersmutation pathway.🤖 Generated with Claude Code
Summary by CodeRabbit