feat(skills): deterministic runtime skill bundles + L1 compact context + insights split by hfm77788 · Pull Request #1 · hfm77788/hermes-agent

hfm77788 · 2026-05-22T23:59:46Z

Summary

Runtime deterministic skill bundles + L1 compact context + insights split

Changes

Deterministic skill bundles (agent/skill_commands.py)
- PR/git → code-task-execution + github-pr-workflow
- debug/root-cause → code-task-execution + systematic-debugging
- TDD/regression → code-task-execution + test-driven-development
- review/preflight → code-task-execution + requesting-code-review
- skill fix → code-task-execution + hermes-agent-skill-authoring + skill-self-repair
- Reduces non-deterministic auto-preload and ensures related skills load together
L1 compact context (agent/skill_commands.py + hermes_cli/config.py)
- Obvious small fixes now get a lightweight SOP injected instead of the full SKILL.md
- New config: skills.auto_preload_compact_l1=True (default)
- Significantly reduces context size and latency for L1 fast-path tasks
Insights split (agent/insights.py)
- Top Skills now distinguishes: view_count (total) / explicit_view_count / auto_preload_count
- Fixes false "Loads=0" confusion where auto-preload was working but not counted
Tests
- tests/agent/test_skill_auto_preload.py: runtime routing tests (62 passing)
- tests/agent/test_insights.py: adds auto_preload_count split test with isolated HERMES_HOME
hermes doctor false-positive removal (hermes_cli/doctor.py)
- Removed the issues.append("Run 'hermes setup' ...") line that merged missing-API-key tool warnings into a single false issue

Verification

python -m pytest tests/agent/test_skill_auto_preload.py tests/agent/test_insights.py -q → 62 passed
~/.hermes/scripts/code-task-preflight.sh standard → READY_TO_COMMIT
No secrets or credentials in diff

Notes

Commit is already pushed to fork/fix/skill-preload-runtime-compact-l1
Branch is ahead of upstream; no conflicts with main

…t + insights split - skill_commands.py: runtime deterministic bundles (PR→github-pr-workflow, debug→systematic-debugging, TDD→test-driven-development, review→requesting-code-review, skill-fix→skill-self-repair); L1 compact context injection for small fixes - hermes_cli/config.py: add skills.auto_preload_compact_l1 default True - agent/insights.py: Top Skills now distinguishes total/view_count vs explicit_view_count vs auto_preload_count - tests/agent/test_skill_auto_preload.py: runtime routing tests (62 passing) - tests/agent/test_insights.py: add auto_preload_count split test - hermes_cli/doctor.py: remove false-positive issues.append for missing API keys

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d727cd370

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-23T00:03:57Z

+        user_msg_content = (
+            f"{auto_skill_context.strip()}\n\n"
+            f"[User request — preserved verbatim below]\n"
+            f"{user_message}"
+        )


Preserve original user text for persisted transcript

This branch prefixes the live user message with auto-preloaded skill content, but the turn does not set a persistence override, so _persist_session() will store the synthetic skill payload as the user's message. In sessions where auto-preload matches, this leaks ephemeral guidance into saved history, inflates context on resume, and contradicts the intended “turn-only” injection behavior.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-23T00:03:57Z

+                        use_count = int(rec.get("use_count") or 0)
+                        if use_count == 0:
+                            continue


Apply time/source filters before using lifetime use_count

This uses .usage.json's use_count directly to compute auto_preload_count, but use_count is cumulative and not scoped by cutoff or source. That means generate(days=...) and source-filtered insights can overcount historical skill loads that occurred outside the selected report window/platform, skewing top-skill rankings.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-23T00:03:57Z

+                        last_used = rec.get("last_used_at")
+                        if last_used and (
+                            entry["last_used_at"] is None
+                            or last_used > entry["last_used_at"]
+                        ):


Normalize timestamp types before last_used comparison

The sidecar last_used_at value is an ISO string (bump_use() writes _now_iso()), while DB-derived last_used_at is numeric epoch. Comparing these directly with > can raise TypeError when a skill appears in both sources, which can break insights generation for otherwise valid data.

Useful? React with 👍 / 👎.

hfm77788

Closing without merge.

Reason:

This PR is stale against current main and is not mergeable.
The runtime skill/preload architecture has evolved since this branch was opened.
The branch should not be merged as-is because it may overwrite or conflict with later runtime changes.

If still needed, rebuild the useful parts as a fresh minimal PR against current main.

* fix(windows): harden gateway scheduled task * fix(windows): launch gateway scheduled task via console-less wscript The Scheduled Task ran the gateway through cmd.exe, which allocates a console. During logon Windows broadcasts CTRL_CLOSE_EVENT to console process groups, reaping cmd.exe and the half-initialized gateway with STATUS_CONTROL_C_EXIT (0xC000013A) - which Task Scheduler treats as a user cancel, so RestartOnFailure never fires and the gateway vanishes on every reboot (issue NousResearch#45599 root cause #1). Add a console-less .vbs launcher (wscript.exe -> pythonw.exe, both GUI-subsystem) mirroring the gateway.cmd env + argv, and point the task action at it. The .cmd stays for the Startup-folder fallback and /Run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Jeff <jeffrobodie@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…eation snapshot (NousResearch#44585) An unpinned cron job follows the global default provider (config.yaml model.default + resolve_runtime_provider). If that global state is changed after the job is created — e.g. a temporary switch to a paid provider like nous/claude-fable-5 — the job silently inherits it on its next tick and spends real money. This is the reported $7.73 incident: a job created under a free/default provider later inherited a temporary paid switch. Fix (ask #1 only) preserves the legitimate "unpinned job should follow model.default" use case by detecting *drift* rather than freezing the model: - create_job (cron/jobs.py): for UNPINNED, agent-backed jobs (no explicit provider, not no_agent), snapshot the provider that resolution WOULD pick right now into a new optional `provider_snapshot` field, resolved via the same resolve_runtime_provider() path the ticker uses. Fail-open to None on any resolution error so job creation never breaks. - run_job (cron/scheduler.py): right after runtime resolution, if the job has a provider_snapshot AND is unpinned AND the currently-resolved provider DIFFERS from the snapshot, fail closed for that run — make no paid call and deliver a loud, actionable alert naming both providers and telling the user to pin explicitly (`cronjob action=update job_id=.. provider=..`). Back-compat: jobs with no snapshot (pre-existing jobs, no_agent jobs, or any job whose creation-time resolution failed) behave exactly as before — the guard only engages when a snapshot exists. Explicitly-pinned jobs (job.provider set) are unaffected since they don't drift with global state. Tests: tests/cron/test_cron_provider_pin.py covers snapshot-matches (runs), snapshot-differs (fail closed, no agent constructed), no-snapshot back-compat, None-snapshot back-compat, explicitly-pinned (runs regardless), plus create_job snapshot capture/skip/fail-open. The fail-closed case is load-bearing (fails without the guard). Issue NousResearch#44585 asks #2-4 (hard-stop a running job, gateway-stop containment, fail-closed on provider mutation) are out of scope for this change.

…ture get_copilot_api_token now returns (api_token, base_url); the auth-remove suppression test still mocked it as a bare string, mis-unpacking into the credential-pool seed path and failing with 'No credential #1'.

…_id signature churn Two independent bugs evicted the cached gateway AIAgent on every turn, preventing the prompt cache from ever warming: 1. Model normalization mismatch: the post-run fallback-eviction check compared _agent.model (stripped in AIAgent.__init__) against the raw _resolve_gateway_model() config string. For vendor-prefixed config on native providers (e.g. 'deepseek/deepseek-v4-pro' vs 'deepseek-v4-pro') this was always unequal, so the agent was evicted after every successful run. Normalize _cfg_model the same way (skip aggregators). 2. Discord triggering message_id leaked into the cached system prompt via build_session_context_prompt()'s Discord IDs block. message_id changes every turn, so the agent-cache signature (computed from the ephemeral prompt) changed every Discord turn -> rebuild every message. The id is now injected per-turn into the user message (where per-turn content belongs and does not touch the cache signature); the cached IDs block carries a static pointer to it, preserving reply/react/pin via the discord tools. Adapted from NousResearch#28846. Bug #1 fix is the contributor's; bug #2 reworked to be non-destructive (keeps the triggering-id capability instead of deleting it). Redundant auto-reset eviction (already on main via NousResearch#9893/NousResearch#48031) and the wrong-premise reset_context_note plumbing from the original PR were dropped. Co-authored-by: Hermes Agent <hermes@nousresearch.com>

… fail on '(empty)' sentinel Two related bugs caused subagent delegation to silently return empty summaries with 0 tokens when the user configured delegation.provider=bedrock alongside delegation.base_url=https://bedrock-runtime.<region>.amazonaws.com. Root cause #1 — misrouting in _resolve_delegation_credentials(): The configured_base_url branch unconditionally forced provider='custom' and api_mode='chat_completions', only specializing for chatgpt.com, anthropic, and kimi hosts. Bedrock (and other native-SDK providers) fell through as 'custom' + chat_completions, which then POSTed OpenAI-shaped JSON at Bedrock's native API. Bedrock rejected the payload and returned nothing, which looked like an empty LLM response to the child agent. Fix: when provider is one of {bedrock, vertex, google, google-genai}, skip the base_url short-circuit and fall through to resolve_runtime_provider(), which knows how to construct the proper SDK client. base_url can still be forwarded through that path for regional overrides. Root cause #2 — '(empty)' sentinel accepted as success: After N retries of empty LLM responses, run_agent.py emits the literal string '(empty)' as final_response. _run_single_child then hit `elif summary:` — '(empty)' is truthy, so status became 'completed' and the parent surfaced a blank result with no error. Users saw api_calls=4, tokens=0, duration~0.4s, status=completed. Fix: treat final_response.strip() == '(empty)' as a failure so the parent surfaces it instead of silently accepting zero-content 'success'. Both paths were reproduced in a live Hermes TUI session on us-west-2 Bedrock (provider=bedrock, model=us.anthropic.claude-sonnet-4-6) and are covered by new tests in tests/tools/test_delegate.py.

…native extension) unicode61 indexes a CJK run as ONE token, so 2-char Korean terms (일본, 구글, 우리, ...) can never match it and the trigram tokenizer needs >=3 chars per term — any query containing a 1-2 char CJK token falls through to a LIKE full-table scan (measured 3-6.4s CPU per query on a 6.8GB production state.db; the #1 base cost behind a 12.4s session_search average on CJK workloads). This ships a ~250-line loadable FTS5 tokenizer (no deps) that wraps unicode61: maximal CJK runs inside its tokens are re-emitted as overlapping character bigrams (Lucene CJKAnalyzer semantics), everything else passes through unchanged. FTS5 phrase semantics turn consecutive sub-tokens into exact substring matching down to 2-char terms at index speed. Build: native/fts5_cjk/build.sh -> ~/.hermes/lib/libfts5_cjk.so (override: HERMES_FTS5_CJK_SO). Salvaged from PR NousResearch#65544; the schema integration lands separately on the v23 external-content layout.

@the3asic

…add same-pid self-reclaim guard Hardening on top of the salvaged dead-PID lease reclamation from PR NousResearch#65775 (@the3asic): - Probe via psutil.pid_exists (hard dependency; CONTRIBUTING.md critical rule #1) with the contributor's os.kill(pid, 0) POSIX probe retained only as a scaffold-phase fallback when psutil is missing. - Same-process holders (pid == os.getpid()) are never probed and never self-reclaimed — another thread's live lease is owned by the lease refresher/release path. - Any probe doubt (exceptions, permission errors) conservatively keeps the lease until normal TTL expiry; Windows stays TTL-only. - Tests: psutil-first dead-pid reclaim (probe call pinned), os.kill fallback path, probe-doubt keeps lease, same-pid no self-reclaim, legacy holder + Windows paths assert NO probe via either API.

…ch#67140) The background write guard decided ownership from `isinstance(usage_rec, dict)`, so a local skill with NO usage record passed. That successful write called bump_patch(), which created a `created_by: null` record — and the identical write was refused from then on. "Allowed exactly once, then never" is a race with our own bookkeeping, not a policy. Reproduced on main: patch #1 succeeds, patch #2 with the same arguments is refused. Option B from the issue. Option A (split `session_review` from `scheduled_curator` and let the session fork patch user-owned skills it consulted) would widen autonomous write permission onto skills the user owns with no user present to consent — wrong direction for a no-user-present actor. - skill_manager_tool: missing and explicit-null records now resolve IDENTICALLY, both fail closed. The refusal names the reason and points at `hermes curator adopt <name>`. - background_review: both review prompts told the reviewer to patch any skill consulted in the session and claimed pinned skills could be improved, while enforcement refused both. Prompts now list pinned, external, and user-owned skills as protected, and tell the reviewer to RECOMMEND adoption instead of attempting a write that will be refused. - skill_usage: document that `created_by` is a curator-management policy flag, not a provenance claim, and add `is_curator_managed()` so call sites read as the question they ask. Field name retained — it is on disk in every `.usage.json` and renaming would strand those records. - curator CLI: `hermes curator list-unmanaged` itemizes unmanaged skills with the reason each is unmanaged (completes the NousResearch#67139 spec). Foreground writes are untouched: a user-directed edit to a user-owned skill still works, including on pinned skills. Sibling tests: 9 failures in test_skill_manager_tool.py were fixtures that created record-less skills to exercise OTHER guards (consolidation-delete, read-before-write) and relied on ownership falling through. Fixed at the fixture, since the real curator only ever operates on managed sediment. One test asserted the old "manually authored" wording; rewritten to assert the behavior contract instead of the string. Validation: 274 targeted tests + all 7 background-review files (60 tests) pass. E2E on a temp HERMES_HOME (30 checks) covers the flip, foreground writes, adoption unblocking, pin semantics, prompt/enforcement parity, and the new verb. Each new test sabotage-verified: revert the fix, confirm it goes red. Fixes NousResearch#67140

…own (NousResearch#74136) Fix-up for the cherry-picked cooldown persistence: the PR's tests mocked the DB (SimpleNamespace(_db=MagicMock())), which cannot prove the cooldown survives a restart. Replace with the production shape — a real SessionDB on disk behind the real AsyncSessionDB facade — and add a restart regression: fail a hygiene compression on runner #1, tear it down, build a fresh GatewayRunner on the SAME database, and assert the cooldown is still honored (no compression agent instantiated). Also updates the timeout test to assert the DB-backed record_compression_failure_cooldown write instead of the removed in-memory dict. Sabotage-verified: reverting gateway/run.py to the in-memory dict makes the restart test fail.

Users following abbreviated links guess /docs/quickstart and /docs/installation and hit raw GitHub-Pages 404s — the real pages live under /docs/getting-started/. Add client redirects for both. Consumer-onboarding audit finding #1, Aug 2026.

The #1 patch failure class in production (state.db mining, 250k-window) is a re-send of an edit that already landed: 'old_string and new_string are identical' (299 occurrences) plus a share of hunk-not-found errors where the new text is already in the file. These errored, sending models into re-read/re-patch loops. New tools/fuzzy_match.is_already_applied(content, old, new) — a conservative check requiring (1) non-trivial new_string (>=8 chars), (2) EXACT presence of new_string, (3) old_string gone (unless identical). Wired into three sites: - patch_replace (replace mode): returns success + no_change: true + an explicit note instead of the identical-strings / no-match error. - V4A validation phase: an already-applied hunk validates as a no-op so multi-hunk patches no longer fail wholesale when one hunk landed in a prior call. - V4A apply phase: mirrors the same skip so the two phases agree. Genuine no-matches (new text absent) and half-applied renames (old text still present) keep their error behavior — covered by tests.

process(action='wait') hitting its window returned status='timeout' with a terse note — models read it as an error and re-issued identical waits (process is the #1 exact-duplicate tool call in production: 511 dupes in a 400k-msg window; wait is 57% of all process actions). The timeout result now carries: - process_running: true — machine-readable 'this is a status, not a failure' - an explicit note: 'Wait window of Ns elapsed — the process is still running. This is not an error. Uptime: Ms.' plus the right next step: when notify_on_complete is set, 'you will be notified on exit — do more work instead of waiting again'; otherwise a pointer to notify_on_complete for next time. - the clamp note (requested > max) now composes with the status note instead of replacing it. Exited/interrupted results are unchanged.

…e-review #1) revoke_commit_admission() used to invoke the holder-qualified lease release unconditionally — including while an admitted commit was still mutating SessionDB — letting a second compressor acquire the durable lock mid-commit and interleave with the first commit's writes. The admission_revoked flag store stays lock-free, but the lease-release decision now coordinates with the fence lock: - revoke acquires the fence lock non-blocking; on success no commit can be in flight (an admitted commit retains the lock until finish_commit) and the release runs immediately, still under the lock so a racing begin_commit cannot slip between the check and the release. - on failure the release is deferred: finish_commit() re-checks _admission_revoked and performs it AFTER the commit completes (prompt even if the worker thread is later parked), and the begin_commit refusal path does the same for a revoke that lost the race to a transient lock-setup/cancel boundary. All paths are idempotent with the worker's own outer cleanup (DB release is holder-qualified). Invariant encoded + tested: no second compressor can acquire the durable lock while an admitted commit is still mutating; after a post-revoke commit finishes the lease is released promptly. Both regressions (revoke-during-commit deferral, revoke-before-commit immediate release + refused begin_commit) are sabotage-verified.

…rst run The first-run provider picker showed Fireworks AI alongside Nous Portal before the user opened the 'Other providers' disclosure. Only Nous Portal should be visible up front; Fireworks now lives inside the expanded list but keeps its #1 position there (Nous -> Fireworks ordering preserved).

chatgpt-codex-connector Bot reviewed May 23, 2026

View reviewed changes

hfm77788 commented Jun 2, 2026

View reviewed changes

hfm77788 closed this Jun 2, 2026

Navigation Menu

feat(skills): deterministic runtime skill bundles + L1 compact context + insights split - #1

feat(skills): deterministic runtime skill bundles + L1 compact context + insights split#1
hfm77788 wants to merge 1 commit into
mainfrom
fix/skill-preload-runtime-compact-l1

hfm77788 commented May 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 23, 2026

Uh oh!

chatgpt-codex-connector Bot May 23, 2026

Uh oh!

chatgpt-codex-connector Bot May 23, 2026

Uh oh!

hfm77788 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hfm77788 commented May 22, 2026

Summary

Changes

Verification

Notes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

hfm77788 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant