Skip to content

fix: scheduler delegation test reads skill_id, not owner - #608

Merged
JarbasAl merged 1 commit into
devfrom
fix/scheduler-owner-skillid-9f21
Sep 16, 2026
Merged

JarbasAl merged 1 commit into
devfrom
fix/scheduler-owner-skillid-9f21

Conversation

@JarbasAl

@JarbasAl JarbasAl commented Sep 8, 2026

Copy link
Copy Markdown
Member

🤖 Auto-generated by Claude Sonnet 5 (claude-sonnet-5) via Claude Code — NOT human-reviewed. Verify before acting.

ovos-bus-client 2.11.14a1 renamed the SCHEDULER-1 record identity key from owner to skill_id. The rename covers the on-disk read boundary in schedules.py's from_stored migration shim, but not the live get()/list() view a consumer actually sees. ovos_workshop has no "owner" reader anywhere in the package, so test_a_delay_in_seconds_becomes_a_relative_timing was asserting a key the current library never produces, and dev has been red on this single line since the bump.

The fix is the test assertion, not the library: record["owner"] becomes record["skill_id"] at test/unittests/test_scheduler_delegation.py line 196.

Bisected the published ovos-bus-client wheels on a clean origin/dev worktree: 2.11.12a1, 2.11.13a1, 2.11.13a3 and 2.11.13a5 all give 44 passed in this file; 2.11.14a1 and 2.11.14a2 give 1 failed, 43 passed with KeyError: owner at the assertion. With the old key in place against 2.11.13a5, the fixed assertion fails as expected (proving the test is real); against 2.11.14a2 the file passes 44/44, and the full test/unittests suite passes 702, with 18 subtests, and zero failures.

This unblocks ovos-workshop#605, whose five red build_tests legs are this same single failure rather than anything in that PR's own diff.

…test

ovos-bus-client 2.11.14a1 renamed the SCHEDULER-1 record identity key
from owner to skill_id in the live get()/list() view a consumer sees;
the from_stored migration shim in schedules.py only covers the
on-disk read boundary, not this view. ovos_workshop has no "owner"
reader at all, so the old assertion was asserting a key the current
library never produces.

Bisected the published wheels on a clean origin/dev worktree: 2.11.12a1,
2.11.13a1, 2.11.13a3 and 2.11.13a5 give 44 passed in this file;
2.11.14a1 and 2.11.14a2 give 1 failed, 43 passed with KeyError owner at
the assertion. After this change: 44 passed in the file and 702 passed
with 18 subtests across test/unittests against ovos-bus-client 2.11.14a2.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the fix label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Hello there! Your PR checks are ready for review. ✨

I've aggregated the results of the automated checks for this PR below.

📋 Repo Health

I've checked the repo's strength (aka test suite robustness). 🏋️‍♂️

✅ All required files present.

Latest Version: 9.7.0a2

ovos_workshop/version.py — Version file
README.md — README
LICENSE — License file
pyproject.toml — pyproject.toml
⚠️ setup.py — setup.py
CHANGELOG.md — Changelog
ovos_workshop/version.py has valid version block markers

⚖️ License Check

I've checked the licenses of all dev-dependencies too. 🛠️

✅ No license violations found.

Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed.

📡 Channel Compat — stable

I've checked the vitals of this contribution. 🩺

🚧 Channel unresolvable with this checkout — the repo's dependency floors exceed what the channel pins (fleet finding; see the install log).

Constraints: https://raw.githubusercontent.com/OpenVoiceOS/OpenVoiceOS/main/constraints-stable.txt
Resolved stack: channel-compat-stable artifact (constraints + pip freeze).

🔒 Security (pip-audit)

Ensuring our digital fortress remains impenetrable. 🏰

✅ No known vulnerabilities found (72 packages scanned).

📡 Channel Compat — testing

Generic report status: Complete and verified. ✅

🚧 Channel unresolvable with this checkout — the repo's dependency floors exceed what the channel pins (fleet finding; see the install log).

Constraints: https://raw.githubusercontent.com/OpenVoiceOS/OpenVoiceOS/main/constraints-testing.txt
Resolved stack: channel-compat-testing artifact (constraints + pip freeze).

🔍 Lint

Evaluating the overall quality of your PR. ✨

ruff: issues found — see job log

🔨 Build Tests

From source to binary, let's see how it holds up. 🧱

✅ All versions pass

Python Build Install Tests
3.10
3.11
3.12
3.13
3.14

Providing clarity through automated analysis 🔍

JarbasAl added a commit that referenced this pull request Sep 8, 2026
OVOS-STOP-1 §4.2 makes `ovos.stop.ping` the stop cascade's poll topic and
treats a handler that does not subscribe to it as `can_handle: false` for
that round. OVOSSkill subscribed only to the pre-spec per-skill
`<skill_id>.stop.ping`, so every skill's stoppability rested on the
compatibility twin ovos-core still emits alongside the broadcast; once that
twin goes, generic stop falls back to the recency rule for the whole fleet.

OVOSSkill now subscribes to both pings and answers on `ovos.stop.pong` with
the §4.2 payload. A core emitting both in one round draws exactly one pong:
STOP-1 §4.2 binds the poll to PIPELINE-1 §4.5, which keys round state by the
`session_id` of `context.session` and by `context.utterance_id` (§9.1.1) —
both propagate to either ping by reply derivation — and rules that the first
valid pong per candidate wins. A pre-spec core that stamps no `utterance_id`
names no round, so its pings are answered one for one as before.

Fail-before: the broadcast test fails on dev (0 pongs, nothing subscribes);
with the subscription but no dedupe the single-pong test fails 2 != 1; a
session-only round key fails two rounds in one session, 1 != 2.
Suite: 708 passed, 1 pre-existing failure in test_scheduler_delegation
(KeyError 'owner', PR #608).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
JarbasAl added a commit that referenced this pull request Sep 8, 2026
OVOS-STOP-1 §4.2 makes `ovos.stop.ping` the stop cascade's poll topic and
treats a handler that does not subscribe to it as `can_handle: false` for
that round. OVOSSkill subscribed only to the pre-spec per-skill
`<skill_id>.stop.ping`, so every skill's stoppability rested on the
compatibility twin ovos-core still emits alongside the broadcast; once that
twin goes, generic stop falls back to the recency rule for the whole fleet.

OVOSSkill now subscribes to both pings and answers on `ovos.stop.pong` with
the §4.2 payload. A core emitting both in one round draws exactly one pong:
STOP-1 §4.2 binds the poll to PIPELINE-1 §4.5, which keys round state by the
`session_id` of `context.session` and by `context.utterance_id` (§9.1.1) —
both propagate to either ping by reply derivation — and rules that the first
valid pong per candidate wins. A pre-spec core that stamps no `utterance_id`
names no round, so its pings are answered one for one as before.

Fail-before: the broadcast test fails on dev (0 pongs, nothing subscribes);
with the subscription but no dedupe the single-pong test fails 2 != 1; a
session-only round key fails two rounds in one session, 1 != 2.
Suite: 708 passed, 1 pre-existing failure in test_scheduler_delegation
(KeyError 'owner', PR #608).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
JarbasAl added a commit that referenced this pull request Sep 8, 2026
OVOS-STOP-1 §4.2 makes `ovos.stop.ping` the stop cascade's poll topic and
treats a handler that does not subscribe to it as `can_handle: false` for
that round. OVOSSkill subscribed only to the pre-spec per-skill
`<skill_id>.stop.ping`, so every skill's stoppability rested on the
compatibility twin ovos-core still emits alongside the broadcast; once that
twin goes, generic stop falls back to the recency rule for the whole fleet.

OVOSSkill now subscribes to both pings and answers on `ovos.stop.pong` with
the §4.2 payload. A core emitting both in one round draws exactly one pong:
STOP-1 §4.2 binds the poll to PIPELINE-1 §4.5, which keys round state by the
`session_id` of `context.session` and by `context.utterance_id` (§9.1.1) —
both propagate to either ping by reply derivation — and rules that the first
valid pong per candidate wins. A pre-spec core that stamps no `utterance_id`
names no round, so its pings are answered one for one as before.

Fail-before: the broadcast test fails on dev (0 pongs, nothing subscribes);
with the subscription but no dedupe the single-pong test fails 2 != 1; a
session-only round key fails two rounds in one session, 1 != 2.
Suite: 708 passed, 1 pre-existing failure in test_scheduler_delegation
(KeyError 'owner', PR #608).

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@JarbasAl
JarbasAl marked this pull request as ready for review September 16, 2026 17:26
@JarbasAl
JarbasAl merged commit 3827467 into dev Sep 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant