Skip to content
This repository was archived by the owner on Jul 22, 2026. It is now read-only.

feat(dashboard): emit bounded slash worker telemetry - #3

Closed
s00rz wants to merge 14 commits into
mainfrom
codex/dashboard-slash-worker-true-north
Closed

feat(dashboard): emit bounded slash worker telemetry#3
s00rz wants to merge 14 commits into
mainfrom
codex/dashboard-slash-worker-true-north

Conversation

@s00rz

@s00rz s00rz commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Draft Status

Opened as draft pending fresh exact-SHA independent review and repository checks for replacement head babf754d34553f46b3a1a6626538bd52c0923d69.

Summary

  • refreshes this same published branch from current fork main with ordinary merge commit 427f7d790071c1579c139a04a9b2dd0fb9803dcf, preserving the fork AUTHOR_MAP entry, current main mappings, profile-home propagation, and telemetry spawn-failure handling
  • makes JSON-RPC IDs correlation-only: every command executes, no response body is cached, and only bounded opaque correlation hashes are retained
  • makes lifecycle and restart telemetry truthful: pipe/EOF/crash/shutdown failures are terminal, worker_stopped requires confirmed exit, bootstrap status propagates, genuine recovery is counted while planned replacement is not, and incident/recovery fingerprints link durably
  • makes queue health deadline-driven and oldest-item based, with deterministic pressure/stall/recovery transitions even without new arrivals
  • bounds restart, dedup (including burst-rejected entries), cardinality, correlation, and diagnostic state by count and TTL
  • keeps telemetry fully fail-open across construction, clock, formatter/logger/sink, worker spawn, command handling, and shutdown
  • wires production command rejection and worker-local response/handoff failures without duplicating gateway/channel delivery or generic process lifecycle
  • proves the real tui_gateway.server logger path reaches dashboard gui.log with adversarial privacy/non-duplication coverage

Kanban idempotency key: t_44c56fb8

Validation

  • uv run --frozen pytest -q -p no:cacheprovider tests/tui_gateway/test_slash_worker_telemetry.py — 24 passed
  • uv run --frozen pytest -q -p no:cacheprovider tests/tui_gateway/test_slash_worker_mcp_discovery.py — 1 passed
  • uv run --frozen pytest -q -p no:cacheprovider tests/tui_gateway/test_slash_worker_profile_home.py — 4 passed
  • uv run --frozen pytest -q -p no:cacheprovider tests/tui_gateway/test_protocol.py — 75 passed
  • uv run --frozen pytest -q -p no:cacheprovider tests/test_tui_gateway_server.py tests/test_tui_gateway_ws.py tests/test_tui_gateway_loop_noise.py tests/test_tui_gateway_queue_on_busy.py — 338 passed
  • uv run --frozen --extra dev ruff check tests/tui_gateway/test_slash_worker_telemetry.py tests/tui_gateway/test_slash_worker_mcp_discovery.py tui_gateway/server.py tui_gateway/slash_telemetry.py tui_gateway/slash_worker.py — passed
  • uv run --frozen python -m py_compile tui_gateway/server.py tui_gateway/slash_telemetry.py tui_gateway/slash_worker.py tests/tui_gateway/test_slash_worker_telemetry.py tests/tui_gateway/test_slash_worker_mcp_discovery.py — passed
  • git diff --check and staged git diff --cached --check — passed
  • one aggregate single-process TUI invocation exposed pre-existing cross-module global/test-state contamination (27 failures after test_protocol.py replaces method registry entries; 664 passed); the affected protocol, profile-home, telemetry, MCP, and server suites above all pass in clean isolated processes

Privacy / Non-duplication Evidence

  • adversarial fixtures assert command arguments, prompts, responses (including a >1 MB response), tokens, authorization values, usernames, home paths, URLs, and raw high-cardinality IDs never enter retained telemetry state or emitted records
  • free-form stderr/exception text is replaced by count/TTL-bounded allowlisted diagnostic classes and redaction markers
  • delivery telemetry records only worker-local response/handoff failure and an opaque gateway-event reference; it does not emit channel-delivery domain events
  • lifecycle telemetry describes only the slash-worker subprocess and never claims stopped while the process remains unconfirmed

Deployment Impact

Code and tests only. No live rollout, runtime/config/channel/token/alert/task/service/queue/process/restart, production telemetry, or collector mutation was performed.

Rollback

Revert babf754d34553f46b3a1a6626538bd52c0923d69 (and the ordinary branch refresh merge if reverting the full remediation generation). Telemetry remains fail-open and command protocol execution does not depend on request-ID replay state.

Agent Notes

  • authoritative repository: s00rz-hermes/hermes-agent
  • branch: codex/dashboard-slash-worker-true-north
  • replacement head: babf754d34553f46b3a1a6626538bd52c0923d69
  • main refresh merge: 427f7d790071c1579c139a04a9b2dd0fb9803dcf
  • same PR and branch; no rebase, force-push, history rewrite, companion PR, rollout, or merge

Merge Readiness

  • Opened as draft
  • PR body complete
  • Validation reported or skipped checks justified
  • Runtime impact understood
  • Focused remediation on the existing PR
  • Correct target repo and branch
  • No unrelated files included
  • Tests or checks reported
  • No operator-only live action performed
  • Exact-SHA independent review clean
  • Required CI checks green

hermes-agent and others added 13 commits July 11, 2026 08:38
* fix(kanban): route not-clean reviews to remediation

* fix(kanban): defer incomplete review routing

* fix(kanban): persist clean review handoffs

* fix(kanban): persist deferred review routing gates

* fix: bind re-review routing to remediation SHA

* chore: register remediation commit author

* fix: gate rereviews on completed remediation

* fix(kanban): preserve deferred review gate on archive

---------

Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
Co-authored-by: Codex <codex@openai.com>
#5)

* fix(kanban): capture worker exit codes on Windows via retained Popen handles

On Windows the dispatcher abandoned each worker's Popen handle at spawn
and reap_worker_zombies() was a no-op, so worker exit codes were never
observed. Every worker death surfaced as an opaque 'pid N not alive'
crash, which made four protections dead code on this platform:

- rate-limit sentinel exits (code 75) were counted as crashes instead of
  requeueing without a failure — quota walls breaker-blocked entire boards
- clean-exit protocol violations were never classified
- real exit codes never reached error text or events
- PID-only liveness probes could confuse a dead worker with an unrelated
  process that recycled its PID (claim extensions on zombies)

Fix, all behind os.name == 'nt' (POSIX behavior unchanged):

- retain each spawned worker's Popen handle in a registry keyed by pid
- reap_worker_zombies() polls retained handles and records returncodes
  into the existing _recent_worker_exits registry
- _classify_worker_exit() reads returncodes directly on nt (no waitpid
  encoding) and falls back to the retained handle when no reap ran
- new _worker_alive() prefers the retained handle over PID probes in
  detect_crashed_workers and release_stale_claims; an open process
  handle also prevents the kernel from recycling the pid at all

Exited handles are retained for the exit-record TTL (so same/next-tick
liveness checks stay handle-backed) and then released.

Tests: 6 new Windows-path tests; _exited_status() test helper made
platform-aware, which also un-breaks the two existing rate-limit tests
on Windows hosts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: overnight-autonomous-2026-07-12

* fix(kanban): persist breaker override limits, add global quota-wall brake and crash backoff

Three dispatcher-hardening fixes from the 2026-07-11 mass-block forensics:

1. Breaker override-limit persistence. The systemic-fingerprint and
   protocol-violation fast paths trip _record_task_failure at
   failure_limit=1, but recompute_ready resolves the limit independently
   (per-task max_retries else the CONFIG limit) and re-promoted every
   override-tripped card in the same tick (cf=1 < 2) into a second
   doomed spawn — inverting the protection. 18 of 28 cascade cards show
   the same-second gave_up->promoted->crash pair. Override trips now
   stamp their effective limit into the task's max_retries
   (persist_limit=True) so both readers agree; the docstring's false
   'never disagree' claim is corrected.

2. Global quota-wall brake. When a worker dies on a provider usage
   wall, every new spawn is doomed until the window resets, but the
   dispatcher kept spawning fresh workers every tick (23 ticks, 64
   crashed sessions on 07-11). A board-wide wall is now recorded in a
   new kanban_meta table — by the worker itself just before exiting
   with the EX_TEMPFAIL sentinel (works even where exit codes are
   invisible to the dispatcher), and by detect_crashed_workers when it
   observes a rate-limited exit — and dispatch_once spawns nothing
   until the wall expires (reclaim/promote bookkeeping still runs).
   resets_at is parsed from the provider error when present (epoch or
   ISO-8601), else defaults to the rate-limit cooldown; walls only
   extend and are capped at 6h.

3. Crash backoff. check_respawn_guard gains a 'crash_backoff' reason:
   after a failed run, respawn waits base * 2^(consecutive_failures-1)
   seconds (HERMES_KANBAN_CRASH_BACKOFF_BASE_SECONDS, default 120s,
   capped 30min, 0 disables) so an outage shorter than the dispatch
   interval can no longer burn a card's whole failure budget.

Tests: 10 new; existing spawn-failure breaker tests explicitly disable
the new backoff (they assert immediate-retry mechanics).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: overnight-autonomous-2026-07-12

* fix(kanban): validate card skills at authoring time; operator-channel auto-subscribe fallback

Two authoring-time protections from the 2026-07-11 board forensics:

1. Skill-roster validation in create_task. Worker spawns force-load the
   card's skills via 'hermes --skills X'; cli.py hard-fails when EVERY
   requested skill is missing from the assignee profile's roster, so a
   decomposer that assigns skills its assignee doesn't have spawn-kills
   the worker at argparse (37 deterministic deaths, breaker-blocked
   cards). create_task now checks the requested skills against the
   assignee profile's installed roster (SKILL.md dirs under the
   profile's HERMES_HOME/skills) and strips unknown names, recording
   them as skills_stripped on the created event. Conservative: when the
   roster cannot be determined (unknown profile, no skills tree) nothing
   is stripped; HERMES_KANBAN_SKILL_VALIDATION=0 disables.

2. Operator-channel fallback in _maybe_auto_subscribe. Cards created
   with no session context (dispatcher, decomposer, cron) previously
   got NO notification subscription — 41 of 43 blocked cards on the
   07-11 board were born silent. When kanban.operator_channel
   ('platform:chat_id[:thread_id]') is configured, such cards now
   subscribe that channel, delivered via kanban.operator_notifier_profile
   (default 'default' — must be a profile with a connected adapter).
   No-op when unconfigured.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: overnight-autonomous-2026-07-12

* fix(kanban): let explicit re-queues bypass the active_pr respawn guard

The active_pr guard defers any card with a GitHub PR URL in a comment
from the last 24 hours, with no bypass. PR-steward cards (refresh /
remediate / un-draft lanes) cite their PR in nearly every comment, so
any lane needing a second spawn - park->release, block->unblock,
reclaim - froze for the full window. On 2026-07-12 this stalled
merge-eligible PRs (CLEAN verdict, all checks green) at the un-draft
hand-off for hours and starved the serial merge train.

Mirror the recent_success guard's existing bypass: an explicit
re-queue event (status, promoted, unblocked, reclaimed) recorded AFTER
the newest PR-URL comment is a deliberate re-run request and allows
the spawn. The anti-duplicate-PR intent is preserved: a card that just
opened a PR and crashed still cannot silently respawn.

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

* fix(kanban): restrict active_pr bypass to operator-driven requeues

Review remediation for the exact-head NOT CLEAN verdict:
- Only reclaim events with payload manual=true bypass (release_stale_claims
  auto-reclaims are the crash-after-opening-a-PR case the guard suppresses).
- Strict > on created_at: integer-second clocks made >= treat a pre-comment
  same-second event as after the comment.
- New tests: real release_stale_claims producer path does not bypass;
  manual reclaim_task does; same-second tie fails closed.

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

* fix(kanban): fail closed on non-literal manual reclaim payloads

Review remediation round 2: the manual-reclaim discriminator accepted
any truthy value and crashed on valid non-object JSON ([] escaped the
narrow except as AttributeError). Require isinstance(dict) plus
payload.get(manual) is True, catch broadly, fail closed. New test
covers truthy-string, numeric, list, and null payloads.

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

---------

Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	scripts/release.py
#	tui_gateway/server.py
@s00rz

s00rz commented Jul 13, 2026

Copy link
Copy Markdown
Owner Author

VERDICT: NOT CLEAN
HEAD: f7499de
REVIEWER: reviewer/t_62c78f52

Exact-head review against base 3e69dacb8990ecfb9681d81921c9b8086b104a8f found these blockers:

  1. tui_gateway/slash_telemetry.py:85-109,297-298 — the claimed allowlist-only summarizer still copies any free-form PascalCase token ending in Error, Exception, or Warning verbatim. Deterministic repro: redact_summary("prompt=CustomerAcquisitionRoadmapError /home/alice/private.txt") returned CustomerAcquisitionRoadmapError;path_redacted;content_redacted, and the marker persisted in the emitted HERMES_TELEMETRY wire record. User-controlled content can therefore survive in memory and GUI-log persistence.

  2. tui_gateway/slash_telemetry.py:350-354 — each class-global restart-history deque is TTL-pruned but has no count cap. At a fixed clock, 5,000 calls to record_restart_attempt() for one session retained all 5,000 timestamps, despite the documented/required strict count bound.

  3. tui_gateway/slash_telemetry.py:282-285,335-395 and tui_gateway/server.py:322-329 — global restart state and incident fingerprints key only on session_ref, omitting profile identity; additionally the production constructor hashes the launch HERMES_HOME instead of its explicit per-session profile_home. Two telemetry instances with the same session key and different profile homes produced the same restart-health fingerprint, and profile B's record_ready_recovery() resolved profile A's incident without B having any restart. This corrupts multi-profile restart accounting/linkage.

  4. tui_gateway/slash_telemetry.py:314-321MAX_WIRE_BYTES is not actually enforced. The code removes only summary and then emits even if the rebuilt record is still oversized. A deterministic call with 200 accepted *_bucket fields emitted an 11,150-byte line with MAX_WIRE_BYTES == 2048.

  5. tui_gateway/slash_worker.py:173,198-199 — bootstrap failure returns 1 from main(), but the module entry point discards that return value instead of exiting with it. A real runpy subprocess with forced HermesCLI construction failure emitted the advertised exit_code: 1 control frame but exited with process status 0.

  6. tui_gateway/server.py:406-412 — stdout EOF emits worker_crashed using a single immediate proc.poll(), so the real exit status is lost when EOF precedes process reaping. Deterministic child repro closed stdout, slept, and exited 23: telemetry observed exit_code=None, then wait() returned 23.

  7. tui_gateway/server.py:605-608,620-653close() sets _closed=True before exit is confirmed. If terminate/wait/kill/wait all fail and the process remains live, it emits worker_shutdown_failed, but every later close() becomes a no-op, permanently preventing retry and allowing the live worker handle to be dropped by the slash.exec recovery path. The terminate/wait/kill/wait repro recorded no additional process calls on a second close().

Validation at this exact head:

  • New telemetry + MCP-discovery focused suite: 25 passed.
  • Broader slash-worker/protocol/goal suite: 102 passed.
  • Ruff on all changed Python files: passed.
  • git diff --check: passed.
  • Seven deterministic negative reproductions above: confirmed.
  • Review used an isolated detached clone with push disabled; no branch write and no live rollout/runtime mutation were performed.

@s00rz
s00rz marked this pull request as ready for review July 14, 2026 01:32
The profile-local MCP probe blocks HermesCLI's one-time tool snapshot on
wait_for_mcp_discovery(), whose bound defaults to 1.5s. A cold CI runner
needs longer than that to spawn a fresh interpreter, import
mcp.server.fastmcp, and finish the stdio handshake, so the snapshot was
taken before 'profileprobe' connected and /tools returned only the
built-ins (slice 4/8 failure on PR #3). thread.join returns the instant
discovery completes, so pinning mcp_discovery_timeout=20 in the test
config costs ~0s locally while giving CI ample headroom. Reproduced the
failure locally by forcing the bound to 0.01s.
@s00rz

s00rz commented Jul 14, 2026

Copy link
Copy Markdown
Owner Author

Superseded — re-homed into hermes-desktop-overlay (patches/current-uncommitted.patch, commit 5cd32f0). The deployment builds from upstream NousResearch/main + the overlay, not this fork. This fork is being reset to upstream and archived; it now exists solely to hold upstream PR NousResearch#61868. Closing.

@s00rz s00rz closed this Jul 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants