fix(telemetry): recompute skills-index split on gateway restore path by Kyzcreig · Pull Request #31 · ANG-Ventures/hermes-agent

Kyzcreig · 2026-06-11T15:48:43Z

Bug

comp_skills_tokens (the System-prompt → Identity/rules + Skill index split from #29) was 0 on every gateway turn (Discord / Telegram / cron) while working correctly on CLI. Empirically confirmed in the blackbox store: 495 rows with comp_skills=0 are all platform IN (discord, telegram, cron); the rows with real values (7210, 6410, …) are cli.

Root cause

The skills substring is stashed on agent._skills_prompt_text only inside build_system_prompt_parts, which runs only when the prompt is built from scratch. The gateway constructs a fresh AIAgent per turn and restores the system prompt verbatim from the session DB (_restore_or_build_system_prompt, the prefix-cache path) — so _build_system_prompt never runs, the stash stays "", and compose_request_breakdown receives skills_prompt="" → comp_skills_tokens=0. CLI uses a long-lived agent that built the prompt once, so its stash persists.

(The #30 message-count + framing features don't have this bug — they derive from api_messages/effective_system at the call site, not from a build-time stash.)

Fix — Option A (stateless recompute)

New _resolve_skills_prompt_text(agent):

Prefers the stash when set.
Else recomputes via the cached build_skills_system_prompt (in-process LRU + disk snapshot — cheap), using the same has-skills-tools gating and avail_toolsets derivation as the real builder.
Memoizes the result back onto the agent for the rest of the turn.
Best-effort: any failure returns the (empty) stash and never raises — telemetry must never break the loop.

The compose call site routes through it. This decouples the telemetry from the build-vs-restore path entirely and self-heals every future gateway turn, no DB migration.

Verification

End-to-end with a gateway-shaped agent (empty stash, skills tools present): recomputes a ~20k-char index → ~5,845 skill tokens (was 0), memoizes it, and identity + skills == sys exactly.

4 new tests: stash-preferred (no recompute), gateway recompute + memoize, no-skills-tools → "", recompute-failure → "" never raises.
14 restore + 19 composition + 56 blackbox pass; ruff clean.

Scope

Display-only; zero effect on the bytes sent, billing, Measured occupancy, or compression.
Sum invariant unchanged.

The System-prompt skill-split (comp_skills_tokens, PR #29) was 0 on every gateway turn (Discord/Telegram/cron) while working on CLI. Root cause: the skills substring is stashed on agent._skills_prompt_text only inside build_system_prompt_parts, which runs only on a from-scratch prompt build. The gateway constructs a fresh AIAgent per turn and RESTORES the system prompt verbatim from the session DB (_restore_or_build_system_prompt), so the stash is never set and compose_request_breakdown receives skills_prompt=''. Fix (Option A): add _resolve_skills_prompt_text(agent) — prefers the stash, else recomputes via the cached build_skills_system_prompt (in-process LRU + disk snapshot, cheap) using the same has-skills-tools gating as the builder, and memoizes the result back onto the agent for the rest of the turn. The compose call site now routes through it. Display-only; best-effort (never raises); decoupled from the build-vs-restore path so it self-heals. Verified end-to-end: gateway-shaped agent (empty stash, skills tools) now recomputes a ~20k-char index → ~5.8k skill tokens (was 0); identity+skills sums exactly to sys. Tests: 4 new cases (stash-preferred, gateway recompute+memoize, no-skills- tools -> '', recompute-failure -> '' never raises). 14 restore + 19 composition + 56 blackbox pass; ruff clean.

github-actions · 2026-06-11T15:49:34Z

🔎 Lint report: `fix/skills-split-gateway-recompute` vs `origin/main`

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9901 on HEAD, 9901 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5118 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

…verclaim Opus code-review BLOCKed the first cut: two invariant tests passed vacuously. Fixed both to be genuine gates, plus the required-changes: - INV-6 (no transcript content): the test was a TAUTOLOGY — it declared a SECRET marker but never seeded it where the PHASE logger could reach it, so the assertion was true by construction. Now seeds the sentinel into the transcript (append_to_transcript), _pending_messages, and the running-agent object, then asserts it never appears in any PHASE= line. MUTATION-PROVEN: adding msg=%s to the log line turns it RED. - INV-3 (fail-open / non-blocking): the old test over-claimed 'non-blocking' with a bound its own docstring admitted didn't prove it. Replaced with an honest single-emit + FAIL-OPEN gate, and made the CODE genuinely fail-open (both PHASE logs wrapped in try/except so a raising/broken sink can't abort the resume-mark — losing the mark = losing the interrupted work). Dropped the overclaiming code comment (handler blocking is a gateway-logging-config concern, not this site's). - RC#3: added a direct reason-UPGRADE test (mark clean restart_consumed → mark interrupted → upgraded to restart_consumed_interrupted) so the overwrite can't silently regress if mark_resume_pending ever gains idempotency. - Residual #31 (reason drift): added a test proving a self-restart reason does NOT leak into a later unrelated shutdown (clear_resume_pending nulls resume_reason + the post-turn gate pops _session_initiated_restart → back to shutdown_timeout). - Test hygiene: the logger-level test now restores the prior level in finally. - Maintainability: promoted the 4-site magic literal to a named module constant _REASON_RESTART_CONSUMED_INTERRUPTED. 199 passed / 1 skipped.

…mb + restart_consumed constant Code-review pass-2 = APPROVE WITH CHANGES (0 blockers; both prior fake-gate BLOCKERs certified genuinely resolved). Two non-blocking RCs landed: - (SRE) PHASE breadcrumbs were logger.info → lost when a deployment runs gateway.run at WARNING+, exactly while diagnosing a bad restart. Made the mark log LEVEL-AWARE: an INTERRUPTED or replay-marked mark (the diagnostic case) and the boot_resume_scheduled breadcrumb now log at WARNING (survive a raised threshold, matching the sibling drain-timeout warning); a routine clean mark stays INFO so normal shutdowns don't spam WARNING. - (Maintainability) promoted the clean-sibling literal 'restart_consumed' to a named constant _REASON_RESTART_CONSUMED alongside its twin _REASON_RESTART_CONSUMED_INTERRUPTED, so the cascade-guard pair moves together. Residual #31 (reason drift) verified against the PRODUCTION path: _apply_post_turn_resume_gate (called post-turn at the run loop) pops _session_initiated_restart AND calls clear_resume_pending (nulls resume_reason) — so the drift the test proves absent cannot ship. Tests: WARNING-level-survives + clean-stays-INFO + caplog-propagation-pin. 209 passed / 1 skipped.

…bservability + reboot-parity proof (#142) * fix(gateway): auto-surface a self-initiated restart that was drain-interrupted (restart_consumed_interrupted) A turn whose last act fires safe-restart.py sets _session_initiated_restart, so on shutdown the F1/F2 cascade-breaker stamps restart_consumed — deliberately EXCLUDED from _AUTO_RESUME_REASONS to break the restart->resume->restart cascade. But if that same turn was ALSO still running when the 180s drain timed out (genuinely interrupted in-flight work), it fell in a seam: the cascade guard suppressed the auto-wake AND #136 preserve-and-prompt never got to surface it, so the session stayed silent until the next inbound message (observed live 2026-06-30 20:30 — Apollo's Discord turn preserved but not surfaced). Fix (restart-only; reboot is unaffected — proven): a new resume_reason restart_consumed_interrupted, stamped when a self-initiated restart is marked at the POST-DRAIN-TIMEOUT site (interrupted=True) rather than the pre-drain speculative site. It IS in _AUTO_RESUME_REASONS (so startup auto-resume proactively surfaces the interrupted work, surface-and-wait, no tool replay), but still records the F2 replay-mark, so a genuine restart->resume->restart loop is still bounded and suspended at threshold within the 300s window. - _resume_reason_for_shutdown_mark gains interrupted kwarg; _self_initiated helper also treats an entry already carrying restart_consumed[_interrupted] as self-initiated. - Only the post-timeout mark site passes interrupted=True (the pre-drain speculative mark, which is cleared on graceful finish, stays interrupted=False). - _resume_reason_phrase maps the new reason to 'a gateway restart'. Reboot parity (Task C, proof-not-code): a self-initiated REBOOT never sets _session_initiated_restart (the detector matches literal safe-restart.py, not safe-reboot.py; reboot-watcher writes no breadcrumb), so it marks shutdown_timeout (already auto-resumes). Reboot inherits #136/#137 preservation via the shared SIGTERM spine unchanged. Tests: 7 cascade unit tests + 1 integration call-site gate (real stop() through the drain-timeout branch) + 2 reboot-parity tests. The integration test is MUTATION-PROVEN: flipping the post-timeout site to interrupted=False turns it RED. Change-detector allow-list test converted to an additive-invariant assertion. Spec: plans/2026-07-01_restart-reboot-continuity-observability-SPEC.md (pass-3 APPROVE, all RCs folded). * feat(gateway): PHASE= observability for the shutdown mark + boot-resume decisions Task B (spec 2026-07-01 restart-reboot-continuity). The gateway shutdown spine already logs phase TIMINGS ('Shutdown phase: drain done at +Xs', #140's 'Executor drain:', 'Auto-resume skipped') — that existing 'Auto-resume skipped' line is what let us diagnose the 20:30 seam. This adds the two per-session DECISION points that were invisible: - PHASE=shutdown_mark key=... reason=... interrupted=... replay_marked=... marked=... at _mark_resume_pending_for_shutdown — makes the restart_consumed vs restart_consumed_interrupted vs shutdown_timeout choice observable per session. - PHASE=boot_resume_scheduled key=... reason=... platform=... when a session is proactively resumed at boot (surface-and-wait). Both use the module logger at INFO (INV-3/D-8: no fsync, no rollover in the hot path — logging must never eat the 180s drain budget) and carry session key + flags ONLY, never transcript content (INV-6). Tests: PHASE-line-emitted, no-content-leak (INV-6), and a slow/blocking-handler latency gate (D-8 — proves the mark path doesn't fan out or block). Watcher-side PHASE= markers (safe-restart watcher.py, safe-reboot reboot-watcher.py) ship in the skills tree. * fix(gateway): address Opus code-review — real INV-6/INV-3 gates, no overclaim Opus code-review BLOCKed the first cut: two invariant tests passed vacuously. Fixed both to be genuine gates, plus the required-changes: - INV-6 (no transcript content): the test was a TAUTOLOGY — it declared a SECRET marker but never seeded it where the PHASE logger could reach it, so the assertion was true by construction. Now seeds the sentinel into the transcript (append_to_transcript), _pending_messages, and the running-agent object, then asserts it never appears in any PHASE= line. MUTATION-PROVEN: adding msg=%s to the log line turns it RED. - INV-3 (fail-open / non-blocking): the old test over-claimed 'non-blocking' with a bound its own docstring admitted didn't prove it. Replaced with an honest single-emit + FAIL-OPEN gate, and made the CODE genuinely fail-open (both PHASE logs wrapped in try/except so a raising/broken sink can't abort the resume-mark — losing the mark = losing the interrupted work). Dropped the overclaiming code comment (handler blocking is a gateway-logging-config concern, not this site's). - RC#3: added a direct reason-UPGRADE test (mark clean restart_consumed → mark interrupted → upgraded to restart_consumed_interrupted) so the overwrite can't silently regress if mark_resume_pending ever gains idempotency. - Residual #31 (reason drift): added a test proving a self-restart reason does NOT leak into a later unrelated shutdown (clear_resume_pending nulls resume_reason + the post-turn gate pops _session_initiated_restart → back to shutdown_timeout). - Test hygiene: the logger-level test now restores the prior level in finally. - Maintainability: promoted the 4-site magic literal to a named module constant _REASON_RESTART_CONSUMED_INTERRUPTED. 199 passed / 1 skipped. * fix(gateway): code-review-2 follow-ups — WARNING-level audit breadcrumb + restart_consumed constant Code-review pass-2 = APPROVE WITH CHANGES (0 blockers; both prior fake-gate BLOCKERs certified genuinely resolved). Two non-blocking RCs landed: - (SRE) PHASE breadcrumbs were logger.info → lost when a deployment runs gateway.run at WARNING+, exactly while diagnosing a bad restart. Made the mark log LEVEL-AWARE: an INTERRUPTED or replay-marked mark (the diagnostic case) and the boot_resume_scheduled breadcrumb now log at WARNING (survive a raised threshold, matching the sibling drain-timeout warning); a routine clean mark stays INFO so normal shutdowns don't spam WARNING. - (Maintainability) promoted the clean-sibling literal 'restart_consumed' to a named constant _REASON_RESTART_CONSUMED alongside its twin _REASON_RESTART_CONSUMED_INTERRUPTED, so the cascade-guard pair moves together. Residual #31 (reason drift) verified against the PRODUCTION path: _apply_post_turn_resume_gate (called post-turn at the run loop) pops _session_initiated_restart AND calls clear_resume_pending (nulls resume_reason) — so the drift the test proves absent cannot ship. Tests: WARNING-level-survives + clean-stays-INFO + caplog-propagation-pin. 209 passed / 1 skipped. --------- Co-authored-by: Apollo <apollo@ang.ventures>

- kanban_db #23: the in-txn takeover audit fires only when the txn changed the target card's guarded columns (status/assignee/priority/session_id), not on any side write (reclaim_refused), so a refused mutation never records or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry fakes, CI slice 13). - kanban_db #44: birth home re-resolved at the TOP of the write txn, before the duplicate / parent checks. - kanban_db #70: no own-claim exception by profile name; without a run id only a lapsed run (claim expired, no live owner) can be closed. - checkout_admission #52: read-check-replace under a per-consumer flock. - discord #30/#31: cancellation recorded before fut.done(); a cancelled disconnect waits for the in-flight flush and still runs every flush. - run.py #36: lock held until the worker write finishes on cancel; every write runs in FIFO order (no skipped-but-reported resets). - ci_overflow_integration #27: documented + tested that a secondary-limit 403 arrives as RateLimited(429) and fails closed. New tests, each red against the previous commit.

… still closes the client (#30/#31, PR967) - #30: command-sync state writes go through _state_write_off_loop, which on cancel waits for the worker-thread write before re-raising, so _post_connect_sync_lock is never released under a live read-modify-write. - #31: a cancel during a shutdown flush skips the remaining flushes but still cancels the bot task and closes the client, then re-raises. Tests red on base: test_cancelled_disconnect_mid_flush_still_closes_client, test_cancel_does_not_release_sync_lock_under_a_live_state_write.

- kanban_db #23: the in-txn takeover audit fires only when the txn changed the target card's guarded columns (status/assignee/priority/session_id), not on any side write (reclaim_refused), so a refused mutation never records or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry fakes, CI slice 13). - kanban_db #44: birth home re-resolved at the TOP of the write txn, before the duplicate / parent checks. - kanban_db #70: no own-claim exception by profile name; without a run id only a lapsed run (claim expired, no live owner) can be closed. - checkout_admission #52: read-check-replace under a per-consumer flock. - discord #30/#31: cancellation recorded before fut.done(); a cancelled disconnect waits for the in-flight flush and still runs every flush. - run.py #36: lock held until the worker write finishes on cancel; every write runs in FIFO order (no skipped-but-reported resets). - ci_overflow_integration #27: documented + tested that a secondary-limit 403 arrives as RateLimited(429) and fails closed. New tests, each red against the previous commit.

… still closes the client (#30/#31, PR967) - #30: command-sync state writes go through _state_write_off_loop, which on cancel waits for the worker-thread write before re-raising, so _post_connect_sync_lock is never released under a live read-modify-write. - #31: a cancel during a shutdown flush skips the remaining flushes but still cancels the bot task and closes the client, then re-raises. Tests red on base: test_cancelled_disconnect_mid_flush_still_closes_client, test_cancel_does_not_release_sync_lock_under_a_live_state_write.

- kanban_db #23: the in-txn takeover audit fires only when the txn changed the target card's guarded columns (status/assignee/priority/session_id), not on any side write (reclaim_refused), so a refused mutation never records or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry fakes, CI slice 13). - kanban_db #44: birth home re-resolved at the TOP of the write txn, before the duplicate / parent checks. - kanban_db #70: no own-claim exception by profile name; without a run id only a lapsed run (claim expired, no live owner) can be closed. - checkout_admission #52: read-check-replace under a per-consumer flock. - discord #30/#31: cancellation recorded before fut.done(); a cancelled disconnect waits for the in-flight flush and still runs every flush. - run.py #36: lock held until the worker write finishes on cancel; every write runs in FIFO order (no skipped-but-reported resets). - ci_overflow_integration #27: documented + tested that a secondary-limit 403 arrives as RateLimited(429) and fails closed. New tests, each red against the previous commit.

… still closes the client (#30/#31, PR967) - #30: command-sync state writes go through _state_write_off_loop, which on cancel waits for the worker-thread write before re-raising, so _post_connect_sync_lock is never released under a live read-modify-write. - #31: a cancel during a shutdown flush skips the remaining flushes but still cancels the bot task and closes the client, then re-raises. Tests red on base: test_cancelled_disconnect_mid_flush_still_closes_client, test_cancel_does_not_release_sync_lock_under_a_live_state_write.

- kanban_db #23: the in-txn takeover audit fires only when the txn changed the target card's guarded columns (status/assignee/priority/session_id), not on any side write (reclaim_refused), so a refused mutation never records or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry fakes, CI slice 13). - kanban_db #44: birth home re-resolved at the TOP of the write txn, before the duplicate / parent checks. - kanban_db #70: no own-claim exception by profile name; without a run id only a lapsed run (claim expired, no live owner) can be closed. - checkout_admission #52: read-check-replace under a per-consumer flock. - discord #30/#31: cancellation recorded before fut.done(); a cancelled disconnect waits for the in-flight flush and still runs every flush. - run.py #36: lock held until the worker write finishes on cancel; every write runs in FIFO order (no skipped-but-reported resets). - ci_overflow_integration #27: documented + tested that a secondary-limit 403 arrives as RateLimited(429) and fails closed. New tests, each red against the previous commit.

…_e9ca7d13)

Kyzcreig merged commit c8ffda4 into main Jun 11, 2026
23 checks passed

Kyzcreig deleted the fix/skills-split-gateway-recompute branch June 11, 2026 15:56

Kyzcreig restored the fix/skills-split-gateway-recompute branch September 21, 2026 10:32

This was referenced Sep 27, 2026

fix(c5): 14 follow-up race instances from the FleetReview retro-backfill (stacked on #1355) #1361

Merged

fix(c6): fail closed on 28 fail-open / false-success gates (FleetReview backfill, t_6500a97a) #1362

Merged

Kyzcreig pushed a commit that referenced this pull request Sep 28, 2026

fix(cron): #31 host-down prefix only on the demoted #alerts target (t…

bd6186c

…_e9ca7d13)

ang-fleet-workers Bot mentioned this pull request Sep 28, 2026

fix: 29 P2 backfill remainder-2 rows (B/C/E/F/G/I + A test proxies) (t_e9ca7d13) #1416

Merged

ang-fleet-workers Bot added a commit that referenced this pull request Sep 28, 2026

fix(cron): #31 host-down prefix only on the demoted #alerts target (t…

6abeaba

…_e9ca7d13)

ang-fleet-workers Bot added a commit that referenced this pull request Sep 28, 2026

fix(cron): #31 host-down prefix only on the demoted #alerts target (t…

43bade2

…_e9ca7d13)

Navigation Menu

Repository navigation

fix(telemetry): recompute skills-index split on gateway restore path - #31

fix(telemetry): recompute skills-index split on gateway restore path#31
Kyzcreig merged 1 commit into
mainfrom
fix/skills-split-gateway-recompute

Kyzcreig commented Jun 11, 2026

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Kyzcreig commented Jun 11, 2026

Bug

Root cause

Fix — Option A (stateless recompute)

Verification

Scope

Uh oh!

github-actions Bot commented Jun 11, 2026

🔎 Lint report: fix/skills-split-gateway-recompute vs origin/main

ruff

ty (type checker)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

🔎 Lint report: `fix/skills-split-gateway-recompute` vs `origin/main`