Skip to content

feat(supervisor): hand back what a tick observed as a TickReport - #13

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
YellowSnnowmann:feat/supervisor-tick-report
Sep 2, 2026
Merged

feat(supervisor): hand back what a tick observed as a TickReport#13
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
YellowSnnowmann:feat/supervisor-tick-report

Conversation

@YellowSnnowmann

@YellowSnnowmann YellowSnnowmann commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Supervisor::tick now returns a TickReport — one SupervisorEvent per thing the cycle observed or did, in order — instead of (). The supervisor already logged all of it, but a log line is not something a host can route: it cannot be filtered into an event log a user reads, or turned into a notification when a server stays down. The one host that consumes this crate drives tick itself and had no way to learn that a probe timed out, that a session was torn down, or that a reconnect had been failing for an hour. That is what tinyhumansai/openhuman#5931 asks it to surface, and this is the crate-side half of it.

The supervisor still publishes no health signal of its own (the module docs' reasoning stands); it hands the observations back and the host decides which of them a user should hear about.

Related issue

tinyhumansai/openhuman#5931 — cross-repo, so no closing keyword. The host-side PR bumps vendor/tinymcp onto this commit and translates the report into that application's domain events and notifications.

API or behavior changes

Additive. Three new public types, re-exported from the crate root: TickReport { events }, SupervisorEvent (#[non_exhaustive]: ProbeAnswered, ProbeTimedOut, TransportDropped, Reconnected, ReconnectFailed, Parked), and ServerRef { server_id, qualified_name, display_name }. Supervisor::tick returns TickReport instead of (); every in-tree caller is in statement position, and there is deliberately no #[must_use] on the report so those stay valid. Supervisor::run drops the report.

Behaviour is unchanged: no log line moves or changes level, the streak rule and backoff are untouched. Reconnected::after_failures is read before the backoff entry is forgotten — that is the one new thing the cycle has to remember, and it is what lets a host tell a session rebuilt within the same cycle (nobody noticed) from a server that had stayed down.

Internal: the reconnect arm of tick moved into attempt_connect, and the timeout half of judge_probe into judge_timeout — the same split judge_probe itself got in #5, for the same reason, and to stay under clippy::too_many_lines.

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check — passes.
  • cargo clippy --all-targets --all-features -- -D warningsfails on one pre-existing error that is not in this diff and fails identically on unmodified main (verified by stashing the change and re-running): unknown lint: clippy::unused_async_trait_impl at crates/tinymcp/src/tinybus_module/service.rs:122. This is the same toolchain artefact fix(supervisor): report what a probe observed instead of asserting a drop #5 recorded (local clippy 1.96 no longer knows that lint name). With only that lint name allowed (-D warnings -A unknown_lints) clippy reports nothing for this diff.
  • cargo build --all-targets --all-features — passes.
  • cargo test --all-features — passes: 678 + 4 + 149 + 12 + 18, 0 failed.
  • .github/scripts/check-file-coverage.sh 90 coverage.json — passes; supervisor/report.rs 100%, supervisor/types.rs 91.86%, supervisor/test.rs 99.2%.

Tests

Nine new tests in registry/supervisor/test.rs, in a new "The report" section, over the existing adjustable fixture plus one new dial (fail_next_list: fail exactly one tools/list, so a probe finds a broken transport and the reconnect that follows can still complete — the shape of the field case in #5931, a transport that hiccups once and is fine a second later). Each branch of the cycle pins what it reports: an answered probe; a kept timeout with its place in the streak; the teardown after a run of timeouts and the refused reconnect behind it; a broken transport and the same-cycle rebuild with after_failures: 0; a failed reconnect with its penalty, and silence inside the backoff window; a recovery with after_failures: 1; a parked server reported once and then quiet; a disabled install and an empty store reporting nothing; ServerRef::from, kind() and server() on every variant.

Deliberately untested: the ProbeOutcome::Missing arm still (unchanged reasoning from #5 — forcing the entry to vanish between the membership check and the probe would test the harness, not the code); its report entry is covered by the every_event_names_its_server_and_its_kind construction instead.

Documentation

registry/supervisor/mod.rs gains a "What it hands back instead" section next to the existing "What it deliberately does not do", and report.rs documents every variant and field. No README or docs/ page describes the supervisor's API today, so none needed updating.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

🤖 Generated with Claude Code

The supervisor keeps servers connected and logs as it goes, but a log line
is not something a host can route: it cannot be filtered into an event log a
user reads, or turned into a notification when a server stays down. The one
host that consumes this crate drives `tick` itself and, with `tick` returning
`()`, had no way to learn that a probe timed out, that a session was torn
down, or that a reconnect had been failing for an hour — which is exactly
what tinyhumansai/openhuman#5931 asks it to surface.

`Supervisor::tick` now returns a `TickReport`: one `SupervisorEvent` per
thing the cycle observed or did, in the order it happened.

- `ProbeAnswered { elapsed }` — the nominal case, reported so a host can
  watch latency drift before a server starts missing the window.
- `ProbeTimedOut { after, consecutive, teardown_after }` — a kept session.
- `TransportDropped { outcome, consecutive_timeouts }` — a session ended;
  the `ProbeOutcome` says why, and a timeout carries the streak that did it.
- `Reconnected { tools, after_failures }` — `after_failures` is read before
  the backoff entry is forgotten, so a host can tell a session rebuilt
  within the cycle that ended it (nobody noticed) from a server that had
  stayed down across cycles.
- `ReconnectFailed { error, failures, retry_in }` and `Parked { error }`.

Every event carries a `ServerRef` (id, qualified name, display name) copied
out of the install, so the report owns its data. The enum is
`#[non_exhaustive]`; the report and the ref are plain structs so a host can
build them in its own tests.

The reconnect arm of `tick` moves into `attempt_connect`, and the timeout
half of `judge_probe` into `judge_timeout`, for the same reason `judge_probe`
was split out before: the decisions are the substance and the loop is
bookkeeping, and `clippy::too_many_lines` agrees. No log line changes.
`run` drops the report — it has no one to hand it to, and everything in it
was logged as it happened. The supervisor still publishes no health signal
of its own; which of these observations a user should hear about is the
host's decision, and now it can make one.

Additive in practice: the only signature change is `tick`'s return type
widening from `()`, and every in-tree caller is in statement position.

Refs tinyhumansai/openhuman#5931

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 6c727c2b-90e0-4043-830e-f22e0d8a8011

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

             $0.0224 · 198,337 in / 2,321 out · 16,618 cached (8%) · deepseek/deepseek-v4-flash, openrouter/openai/text-embedding-3-small, z-ai/glm-5.2 · 737 embedded
critique:    $0.0094 · 77,832 in  / 855 out   · 8,090 cached (10%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
security:    $0.0090 · 77,706 in  / 881 out   · 8,528 cached (11%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
tests:       $0.0020 · 22,853 in  / 117 out   · 0 cached (0%)      · deepseek/deepseek-v4-flash
description: $0.0014 · 15,448 in  / 82 out    · 0 cached (0%)      · deepseek/deepseek-v4-flash

@tinysweeper

tinysweeper Bot commented Sep 2, 2026

Copy link
Copy Markdown

How this change flows

4 changed behaviours across 16 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 34 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["ServerDials<br/>changed"]:::changed
  n1["...nswers_with_an_error_is_torn_down_at_once<br/>changed"]:::changed
  n2["serve_adjustable_server<br/>changed"]:::changed
  n3["Supervisor<br/>changed"]:::changed
  n4["new"]:::impacted
  n5["tick"]:::impacted
  n6["connected_to"]:::impacted
  n7["install"]:::impacted
  n8["supervisor"]:::impacted
  n9["insert_server"]:::impacted
  n1 -->|calls| n2
  n1 -->|tests| n2
  n1 -->|calls| n4
  n1 -->|tests| n4
  n1 -->|calls| n5
  n1 -->|tests| n5
  n1 -->|calls| n6
  n1 -->|tests| n6
  n2 -->|uses| n0
  n2 -->|calls| n4
  n6 -->|calls| n4
  n6 -->|calls| n7
  n6 -->|calls| n9
  n7 -->|calls| n4
  n8 -->|uses| n3
  n8 -->|calls| n4
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading

Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge.

tinysweeper 0.1.0

YellowSnnowmann added a commit to YellowSnnowmann/openhuman that referenced this pull request Sep 2, 2026
…nreleased

The host now compiles the MCP contract against tinyhumansai/tinymcp#13
(Supervisor::tick returns a TickReport, needed by tinyhumansai#5931) while the registry
keeps the published v0.3.2 artifact. The module pin gate rightly flags the two
pins as describing different releases; this declares the exact drift with its
reason, the way the tinyruntime entries already do.

The drift is compile-only: the tinymcp module is registry-entered but not
wired (AGENTS.md, "step two of the extraction"), so no build downloads or
loads the artifact. Delete the entry when tinymcp cuts its next release and
the registry pin moves onto it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@M3gA-Mind
M3gA-Mind merged commit 8b0627d into tinyhumansai:main Sep 2, 2026
11 checks passed
YellowSnnowmann added a commit to YellowSnnowmann/openhuman that referenced this pull request Sep 2, 2026
tinyhumansai/tinymcp#13 is merged, so the gitlink moves from the PR head to
the merge commit and the pin exemption records the new `git describe`. The
tree is byte-identical to what was tested, so nothing rebuilds.

The exemption stays until tinymcp cuts a release: the drift is compile-only,
because the tinymcp module is registry-entered but not wired.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants