SESSION-2: add ovos.session.sync and session mutation discipline#36
Conversation
…pline - §2.6: add session mutation discipline rule — handlers SHOULD NOT mutate session unless necessary or prescribed by another spec; forward-ref §2.7 for out-of-utterance propagation - §2.7 (new): define ovos.session.sync — explicit broadcast for session updates outside the utterance lifecycle; consumer obligations for orchestrator (MUST merge for default session) and clients (SHOULD merge for named sessions); SHOULD NOT emit gratuitously - §6.2: orchestrator MUST merge ovos.session.sync for default session - §6.3: component SHOULD NOT mutate session gratuitously; MUST use ovos.session.sync when out-of-utterance propagation is needed - §7 (new): bus topics table with ovos.session.sync - §8: renumber former §7 Non-goals; fix stale §5.2 cross-ref to §5.3 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ovos.session.sync must be merged into the working session snapshot immediately on receipt; the merged state MUST be reflected in the handler-lifecycle .complete and ovos.utterance.handled terminal events. Remove the incorrect 'MUST NOT apply to in-flight utterance' rule — the sync arrives at a handler boundary (§2.6) and must flow forward. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- §2.3: remove duplicate sentence - §5.1: fix stale PIPELINE-1 §5.2 cross-ref → §4.2 - §2.7, §6.3: MUST → SHOULD for ovos.session.sync usage; emitting any session-carrying Message is also valid per §2.6 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 41 minutes and 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughOVOS-SESSION-2 is updated to formalize session mutation discipline and introduce ChangesSession Mutation Discipline and Out-of-Utterance Synchronization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 `@ovos-session-2.md`:
- Line 627: In the "Non-goals" paragraph referencing restart semantics, update
the incorrect section cross-reference: change the pointer from "§5.3" to "§5.2"
so that the mention of "restart-loss" correctly links to the section where
restart semantics are defined; locate the string "restart-loss" or the sentence
mentioning "§5.3" in the Non-goals text and replace the section number with
"§5.2".
- Around line 319-327: When an ovos.session.sync arrives for a named session_id
but there is no in-flight (transient) per-utterance session, add a normative
rule: the orchestrator MUST NOT apply the sync to the default-session store
(session_id == "default") and MUST enqueue the sync for that named session_id to
be merged into the next transient per-utterance working snapshot when it is
created; the orchestrator SHOULD also broadcast the received ovos.session.sync
to other components/consumers immediately for inspection, but only actually
merge into the working snapshot when the next in-flight utterance for that
session_id exists (FIFO ordering if multiple syncs arrive).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| - The **orchestrator** MUST merge a received | ||
| `ovos.session.sync` into its working session snapshot for | ||
| the affected `session_id`. The merge follows §5.1's | ||
| field-replacement rule: present fields in the synced | ||
| snapshot replace current values; absent fields leave current | ||
| values unchanged. For `session_id == "default"` the working | ||
| snapshot is the default-session store (§5); for named | ||
| sessions it is the transient per-utterance session in | ||
| progress (§2.2). The orchestrator MUST reflect the merged |
There was a problem hiding this comment.
Define orchestrator behavior when ovos.session.sync arrives with no in-flight named-session utterance.
Current text says components MAY emit sync “at any time” (Line 301), and orchestrator MUST merge on receipt, but for named sessions the “working snapshot” is only the transient in-progress utterance snapshot. This leaves a normative gap for syncs received when no utterance is active for that session_id, and different implementations can diverge.
Please add an explicit rule (e.g., ignore-for-orchestrator-state but still broadcast-consumable, queue-until-next-utterance, or reject) to keep statelessness and conformance testable.
Also applies to: 537-541
🤖 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 `@ovos-session-2.md` around lines 319 - 327, When an ovos.session.sync arrives
for a named session_id but there is no in-flight (transient) per-utterance
session, add a normative rule: the orchestrator MUST NOT apply the sync to the
default-session store (session_id == "default") and MUST enqueue the sync for
that named session_id to be merged into the next transient per-utterance working
snapshot when it is created; the orchestrator SHOULD also broadcast the received
ovos.session.sync to other components/consumers immediately for inspection, but
only actually merge into the working snapshot when the next in-flight utterance
for that session_id exists (FIFO ordering if multiple syncs arrive).
| See §1 for the full list of non-goals. This section adds one | ||
| clarification: **default-session persistence across orchestrator | ||
| restart** is not defined here. §5.2 makes restart-loss | ||
| restart** is not defined here. §5.3 makes restart-loss |
There was a problem hiding this comment.
Fix incorrect section cross-reference in Non-goals.
Line 627 points to §5.3 for restart-loss, but restart semantics are defined in §5.2. This should be updated to avoid implementer confusion.
🤖 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 `@ovos-session-2.md` at line 627, In the "Non-goals" paragraph referencing
restart semantics, update the incorrect section cross-reference: change the
pointer from "§5.3" to "§5.2" so that the mention of "restart-loss" correctly
links to the section where restart semantics are defined; locate the string
"restart-loss" or the sentence mentioning "§5.3" in the Non-goals text and
replace the section number with "§5.2".
ovos.session.sync carries the updated session in Message.data.session (explicit payload); Message.context.session remains the ambient routing carrier per MSG-1. Add data shape table. Update consumer obligations to reference data.session explicitly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ensures routing metadata is preserved so the sync reaches remote clients through layer-2 transports. Without .forward the message carries no routing context and is invisible to satellite/gateway clients. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consistent with STOP-1 §4.2 and CONVERSE-1 §4.2 — all request/response round-trips now explicitly name the MSG-1 derivation to ensure routing metadata is preserved through layer-2 transports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New section between the single-flip routing model (§3.1.1) and no- central-correlation (now §3.1.3): explains when to use .forward (same direction as the source dispatch) vs .reply (back toward the requester), why the distinction matters for layer-2 transports, and a cross-spec table showing consistent application across all five relevant emissions. Renumber former §3.1.2/§3.1.3 to §3.1.3/§3.1.4; update intro and cross-refs accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cross-spec table of every normative .forward / .reply usage with the decision rule. Companion to patterns.md §3.1.2 narrative. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
ovos.session.sync— a dedicated broadcast topic for propagating session updates outside the normal utterance lifecycle. Components SHOULD use it when no in-utterance emission is available to carry the update. Orchestrator MUST merge it into the working session snapshot and reflect the result in the handler-lifecycle.completeandovos.utterance.handledterminal eventsovos.session.syncovos.session.syncfor out-of-utterance propagationovos.session.syncMotivation
Several specs (STOP-1 §4.4, CONVERSE-1 §4.5) describe handlers opportunistically removing themselves from session lists (
active_handlers,converse_handlers) without a defined mechanism for broadcasting that update when no other Message is being emitted.ovos.session.syncformalises this pattern and gives the orchestrator a clear obligation to honour it in terminal events.Test plan
ovos.session.syncshape: carriesMessage.context.sessionper MSG-1,session_ididentifies the target session.completeand.handled🤖 Generated with Claude Code
Summary by CodeRabbit