fix(cli): do not report tip behind-count on diverged branches - #68677
fix(cli): do not report tip behind-count on diverged branches#68677smfworks wants to merge 237 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the misleading full-clone count; current main still returns HEAD..origin/main directly at hermes_cli/banner.py:255-263, so the premise is valid.
Problems
hermes_cli/banner.py:273only checks ancestry whenbehind > 0. A diverged branch can have a zeroHEAD..origin/maincount; this patch would return0, and the version path consequently printsUp to date(hermes_cli/main.py:4710-4718). Please check ancestry for every successful full-clone count and add the zero-count divergence test.UPDATE_DIVERGEDchanges a shared result contract.tui_gateway/server.py:4738-4742forwards it, but Ink renders an update row only forupdate_behind > 0(ui-tui/src/components/branding.tsx:467-479). Handle the new state there as well; the dashboard contract currently documents only-1(hermes_cli/web_server.py:4144-4150).
Suggested changes
- Carry the divergence state through all consumers of
check_for_updates()and cover each affected rendering contract.
Automated hermes-sweeper review.
| # onto origin/main. Otherwise a diverged feature branch can | ||
| # print a tiny tip count ("1 commit behind") while the trees | ||
| # differ by hundreds/thousands of files (#68484). | ||
| if behind > 0 and not _git_is_ancestor("HEAD", "origin/main", repo_dir): |
There was a problem hiding this comment.
This leaves a diverged branch with HEAD..origin/main == 0 classified as up to date. The stated sentinel contract is ancestry-based, so run the ancestry check for every successful full-clone count and add the zero-count divergence regression case.
440b92a to
a0f0ca4
Compare
Sweeper response — salvageability=mediumHead: (see latest push)
Verification |
Verification Summary (adversarial 6-check — PASS after sibling fix)Head: (latest push)
Tests: Python 4 passed · ruff clean · desktop vitest not runnable in bare worktree (no node_modules); pure TS change + unit test added |
SummaryOne PR, #68677, addresses #68484. Its diff replaces misleading tip-only counts with ancestry-based divergence detection across CLI, TUI, web, and Desktop consumers, including positive- and zero-count regressions, while the issue’s diff-shortstat reporting and Windows Desktop/venv lock preflight are not implemented. Related pull requests
Suggested consolidationKeep #68677 open with a salvage path: preserve its tested divergence detection and end-to-end consumer handling as the best existing partial fix, while requiring the closes relationship to account for the still-open diff-shortstat and Windows Desktop/venv lock-preflight scope, either in this PR or concrete follow-up work. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I68484(["issue #68484 (open)"])
P68677["PR #68677 (open)"]
P68677 -->|best fix| I68484
class I68484 open
class P68677 open
class P68677 best
class P68677 target
click I68484 "https://github.com/NousResearch/hermes-agent/issues/68484"
click P68677 "https://github.com/NousResearch/hermes-agent/pull/68677"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 7 kB of issue/PR text, 3 kB of discussion (5 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Stewardship ping — verified this fix is still needed on current
The PR replaces tip-only counts with ancestry-based divergence detection across CLI, TUI, web, and desktop consumers (including |
a1d6a84 to
2cc063f
Compare
Stewardship follow-up — rebased onto current
|
2cc063f to
5ca61b7
Compare
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
…of band The desktop settings page saves approvals.mode through REST PUT /api/config (and the raw editor through PUT /api/config/raw). Enforcement follows the file immediately, because the approval gate re-reads config per command, but every live session's YOLO/approval indicator repaints only on a session.info event, and the REST save emitted nothing. The indicator showed bypass OFF while approvals.mode=off silently auto-approved every dangerous command, and switching sessions repainted the stale cached per-session state, making the toggle look like it flipped itself back. The gateway /approvals slash command had the same gap. The config.set RPC handler already re-emits session.info to all live sessions after a mode flip; give the other writers the same contract: - tui_gateway/server.py: add broadcast_session_info(), which snapshots _sessions under _sessions_lock and re-emits via _emit_session_info_for_session. Also call it from the /approvals slash mirror when a mode argument was persisted (bare /approvals is read-only). - hermes_cli/web_server.py: after a REST save that actually changed the normalized approvals.mode, call the broadcast through a sys.modules guard (no gateway imported means no sessions to notify). The comparison runs on the in-memory documents (existing vs merged, parsed vs raw): the settings page PUTs the defaulted GET record while disk holds sparse YAML, so a block-level compare would broadcast on every autosave, and re-reading through the config cache after the save could serve the pre-save document on an (mtime_ns, size) key collision. Own-profile saves only: a profile-scoped save targets a different HERMES_HOME than this process's gateway sessions. No broadcast on saves that leave the effective mode unchanged, so settings autosave churn (skin, font, TTS) can't spam session.info. Scope: reaches sessions of the in-process gateway (hermes serve / hermes dashboard, the topologies the desktop app talks to). A spawned tui_gateway.entry child gateway has its own process and _sessions; its TUI statusbar reconciles each turn via the existing session.info emissions.
…-fields fix(openai): cover nested sparse response fields
5ca61b7 to
afb002a
Compare
|
Restored onto current |
|
Partial overlap heads-up: PR #86257 (merged) landed the behind-count class fix — the SSH-official and shallow paths no longer fabricate counts, and |
…anel-remote-download fix(desktop): let Files panel download remote backend files
require.resolve returns the macOS realpath (/private/var/...) while os.tmpdir() stays on the /var symlink, so a raw join() deepEqual failed even though the spawn was correct.
…ktop-rebuild-status fix(update): don't report success when the Desktop rebuild failed (supersedes NousResearch#88359, NousResearch#87984)
…covery-command fix(desktop): recover missing get-windows binding
…oup-icon-fix fix(desktop): close the blank hole in the Sessions header
…-regain-atomic-repaint fix(tui): heal focus regain without a separate screen clear (supersedes NousResearch#88596)
Supersedes NousResearch#75649, NousResearch#86637, NousResearch#87672, NousResearch#88642. Fixes NousResearch#86601 Fixes NousResearch#87652 Fixes NousResearch#87823 Co-authored-by: Charmmy <lilShawtty@qq.com> Co-authored-by: chelsealong <chelsealong@126.com> Co-authored-by: Olympusbuildz <Olympus.roots@outlook.com> Co-authored-by: Ricardo Mendes <ricardo.mendes@maiolabs.ai>
Three local-environment leaks made tests red locally while green on CI: - tests/conftest.py: blank HERMES_REAL_HOME and TERMINAL_HOME_MODE per test. The terminal tool injects both into subprocess envs, so any pytest run launched from a Hermes session inherits them and the hermes_constants home-resolution helpers prefer HERMES_REAL_HOME over the monkeypatched HOME (4 failures in test_subprocess_home_isolation.py). - test_modal_sandbox_fixes.py: reset the import-time _YOLO_MODE_FROZEN flag and pin approval mode to manual in _isolate_approval_state(). HERMES_YOLO_MODE=1 in the launching shell froze True at collection time and every guard auto-approved (2 failures). - test_noninteractive_git.py: strip GIT_ASKPASS/VS Code askpass vars in the fail-fast clone E2E. noninteractive_git_env() intentionally keeps a working askpass helper, but this test asserts the no-helper path; under VS Code the helper blocks on the editor until the 30s timeout (1 failure). Verified: all 49 tests in the three files pass both in a plain dev shell (with HERMES_YOLO_MODE=1, HERMES_REAL_HOME, and VS Code askpass set) and inside an unshare -rn network namespace.
… video pickers Same bug class as the plugin image picker crash (NousResearch#77238): an unguarded `current_model = default_model` fallback that can index the catalog with a key it doesn't contain when the provider's default drifts from its catalog. Applies the `default if default in catalog else next(iter(catalog))` guard to _configure_imagegen_model and _configure_videogen_model_for_plugin.
…e-image-2.0 selectable - plugins/image_gen/xai: merge the live /v1/image-generation-models catalog (5-min cache, 10s timeout, static-table fallback when offline/unauth) into the picker so new xAI Imagine models appear automatically the day they launch, with generic metadata until curated text is added. - Add grok-imagine-image-2.0 to the curated static table (typography/ layout-aware model, API-available since Aug 8 2026). - Edits honor an explicitly selected image-input-capable model (e.g. grok-imagine-image-2.0) instead of always forcing grok-imagine-image-quality; quality remains the default edit baseline. - Tests: hermetic autouse fixture keeps unit runs offline; new coverage for live-merge, unknown-future-model selection, offline fallback, and edit-model resolution. Docs model table updated (en + zh-Hans). Live-verified: /image-generation-models returns grok-imagine-image, grok-imagine-image-2.0, grok-imagine-image-quality; real generation with 2.0 succeeded end to end.
This reverts commit 0f73adb.
Adds xai/grok-imagine-image/v2.0/text-to-image with edit_endpoint xai/grok-imagine-image/v2.0/edit (max 3 reference images). 1k/2k resolution, low/medium quality (pinned 1k+medium = $0.06/image), 13 aspect ratios (we map the standard 3), no seed param in the schema. upscale=True (1k native sub-2MP). Schema verified against fal.ai llms.txt + OpenAPI.
…stores 1.4.2) nanostores 1.4.0-1.4.1 annotate batch() @__NO_SIDE_EFFECTS__. Rollup (via vite build) honors that and erases a result-unused batch(...) call as dead code -- callback included. Since d57f94a/053eb7aab/4e520f085 moved the gateway-switch publication (activate() + + ) inside batch(), packaged desktop builds lost the entire publication: clicking a profile in the rail did nothing at all. Dev builds and vitest run unminified, so only the packaged app broke. nanostores 1.4.2 removes the annotation from batch() (it stays on the creation functions, where it is correct). Bump all three pinned copies (apps/desktop, apps/bootstrap-installer, ui-tui) and add a regression test asserting the installed nanostores never re-annotates batch.
…Research#88404) Adapter ingress derives a session key BEFORE the runner stamps source.profile in _make_profile_message_handler, so the namespace fell back to the active profile and every bot in a multiplexed gateway produced agent:main:<platform>:<chat>. A Telegram private chat reports the user's own id as chat.id, identical for every bot, so two profiles sharing one human collapsed onto a single lane: _pending_text_batches, _active_sessions, the busy-session guard and _post_delivery_callbacks are all keyed on that string. A day of production logs across two bots shows 60 flushes, none carrying the secondary profile's namespace. set_owner_profile records credential ownership on the adapter and _session_key_profile resolves the namespace as source.profile -> _owner_profile -> the session store's resolver, so a secondary adapter keys into its own namespace even before the source is stamped. Stamped sources keep priority, so relay/connector ingress, which routes per event rather than per credential, is unchanged. _configure_profile_adapter installs the owner alongside the other handlers, covering startup and reconnect. Every candidate is type-checked as a non-blank str, and every attribute read goes through getattr: adapters are routinely built without BasePlatformAdapter.__init__, and a duck-typed session store returns a truthy non-string that would otherwise be interpolated into the key as agent:<MagicMock ...>:. Also routes the four call sites that passed no profile at all (feishu media batches, raft, slack _session_key_for_source, telegram photo batches) through the same resolver. test_multiplex_busy_input_mode's secondary-adapter busy case seeded _active_sessions with the unstamped agent:main: key, asserting the pre-fix collapse. It now seeds the lane the profile-owned adapter actually derives. A primary adapter has no owner and an unstamped source, so it resolves exactly as before; with multiplex_profiles off the resolver returns None and every key is byte-identical to today's.
…t model; xAI edits honor dispatched model - plugins/image_gen/openrouter: list_models() now queries the endpoint's /models catalog filtered to output_modalities containing "image" (per-backend 5-min cache, 10s timeout, static 2-model chain as offline fallback; openrouter/auto* router pseudo-models excluded). Every image model OpenRouter serves — including future releases — is selectable in `hermes tools` with no code change. Applies to Nous Portal too via the shared provider class. - plugins/image_gen/xai: forward the dispatched model kwarg into _resolve_edit_model() so an explicitly selected edit-capable model is honored on /images/edits (extends the salvaged NousResearch#55893 fix to the edit path; text-only models still fall back to quality). - Tests: OpenRouter live-catalog filtering/exclusions/order, offline fallback, cache single-fetch; xAI edit-kwarg forwarding incl. the text-only-hijack negative case. Live-verified against openrouter.ai: 9 image-output models returned and rendered, matching the public models?output_modalities=image listing.
… (0-job startup_failure on every dispatch; identical content dispatches fine under a new path, proven by probe PR NousResearch#89894)
This reverts commit ab173e2.
…4, and failed installs surface
Three fixes for the "Install on this agent" pipeline, covering the whole
split-brain class between action-spawning endpoints and their status polls:
1. electron/connection-config.ts — the /api/actions/{name}/status poll family
now routes to the same backend as every action-spawning route. Before,
POST /api/skills/hub/install ran on the PRIMARY backend (scoped route)
while the follow-up status poll for a non-default profile routed to the
profile's POOLED backend, which never registered the dynamic action name
(skills-install-<slug>-<hash> lives only in the spawning process's
memory) -> 404 "Unknown action" toast even though the install succeeded.
POST /api/mcp/catalog/install joins the scoped table for the same reason.
2. src/store/hub-actions.ts — a non-zero subprocess exit now rejects with the
action log tail so the caller's catch toasts it. Before, a failed install
(scan gate, network, bad identifier) stopped silently: no toast, no row
flip, and the unchanged skills list read as "install did nothing".
3. src/contrib/runtime-loader.ts — a disk plugin copy shadowed by a bundled
twin now publishes a visible "(stale disk copy)" inventory row carrying
the folder path, instead of a console.info nobody sees. Stale
desktop-plugins/ leftovers from dev deploys are the same folders that
actively break the feature on shells without the bundled twin.
…ween running bots
resolveStoredSession never probed the ACTIVE profile: the unscoped
/api/sessions GET routes to the PRIMARY backend (not the active
gateway's), and the cross-profile probe loop explicitly skipped the
active key. A hidden Bot Mode canonical chat — never present in the
sidebar cache — owned by the focused bot therefore resolved to
undefined on every switch. The transcript prefetch then went unscoped
to the primary backend, 404'd, and the thread painted empty until the
user opened the session explicitly via right-click → Sessions (which
seeds the cache with a profile-stamped row).
Probe the active profile first in the by-id ladder, so hidden and
uncached sessions on the focused profile resolve with ownership and the
prefetch routes to the owning backend.
Live-repro'd headless via CDP with 3 bot profiles mid-turn: before the
fix every focus-switch painted a blank thread ('Waking up <bot>…');
after, the full transcript paints. Reported by @tbkbossswaglord.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…t a tip count NousResearch#86257 stopped fabricating behind-counts for shallow clones, but a full clone on a diverged branch still reported a misleading tiny tip count (or 0, read as up-to-date) because neither tip is an ancestor of the other (NousResearch#68484). Producer side (was missing — the sentinel was consumed but never emitted): - banner.py: _git_is_ancestor helper; run the ancestry check for every successful full-clone rev-list count and return UPDATE_DIVERGED when neither tip is an ancestor (covers behind>0 and behind==0 divergence). - _format_update_notice + cmd_version render a non-fast-forward warning. - update-count.ts: resolveBehindCount takes headIsAncestorOfTarget and returns -2 on divergence; main.ts computes both ancestry directions. Consumer side (kept in lockstep): - version-status.ts, updates.ts (3 sites incl. mapBackendCheck coercion), about-settings.tsx, updates-overlay.tsx treat -2 as an update signal, never as (+-2) or up-to-date. Tests: 4 CLI (test_banner_git_state.py), 3 electron (update-count.test.ts), 1 frontend (version-status.test.ts). All green.
Replaces the copy-pasted `-2` with a single exported constant so the divergence semantics live in one place (version-status.ts), per review. - Export UPDATE_DIVERGED from lib/version-status.ts and reference it internally for the diverged check. - updates-overlay, about-settings, and the updates store now import the constant instead of re-hardcoding -2. - Clarify the intentionally-tricky double-negative in maybeNotifyUpdateAvailable and pin the export with a behavioral test. - 334 renderer + 22 electron update-count tests pass. No behavior change: the sentinel value is identical. Closes/addresses review feedback on NousResearch#68677.
Summary
On a diverged feature branch,
git rev-list --count HEAD..origin/maincan still print a tiny tip count (e.g.1 commit behind) even when HEAD andorigin/mainare mutual non-ancestors and the trees differ by hundreds/thousands of files. The banner andhermes --versionthen recommend a casualhermes updateas if it were a small fast-forward.Fix
UPDATE_DIVERGED = -2sentinelgit merge-base --is-ancestor HEAD origin/mainbefore reporting an exact behind numberUPDATE_DIVERGEDinstead of the misleading tip countbranch diverged from origin/main — not a fast-forward; review before hermes update (switches to main / may stash WIP)UPDATE_AVAILABLE_NO_COUNT)Shallow clones and pure fast-forward behind counts are unchanged.
Out of scope (follow-ups from #68484)
Desktop
release/win-unpackedlock planner / pack-to-release-verifypath — separate PR.Test plan
1→UPDATE_DIVERGED(not1)Closes #68484