fix(desktop): don't persist zoom fallback on a transient read failure - #1
Merged
Conversation
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
# Conflicts: # hermes_cli/observability/shared_metrics_contract.py # hermes_cli/observability/shared_metrics_subscriber.py # scripts/smoke_nemo_relay_shared_metrics.py # tests/hermes_cli/test_plugins.py # tests/hermes_cli/test_relay_shared_metrics.py # tests/hermes_cli/test_relay_shared_metrics_runtime.py # tests/run_agent/test_run_agent.py # tests/test_model_tools.py # tests/tools/test_approval.py
Signed-off-by: Alex Fournier <afournier@nvidia.com> # Conflicts: # hermes_cli/observability/schemas/hermes.shared_metrics.v1.schema.json # scripts/smoke_nemo_relay_shared_metrics.py # tests/agent/test_skill_commands.py # tests/hermes_cli/test_relay_shared_metrics.py # tests/hermes_cli/test_relay_shared_metrics_runtime.py # tests/tools/test_skill_manager_tool.py # tests/tools/test_skill_usage.py # tests/tools/test_skills_tool.py
Signed-off-by: Alex Fournier <afournier@nvidia.com> # Conflicts: # docs/observability/relay-shared-metrics.md # hermes_cli/observability/shared_metrics.py # tests/hermes_cli/test_relay_shared_metrics.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com> # Conflicts: # hermes_cli/observability/schemas/hermes.shared_metrics.v1.schema.json # hermes_cli/observability/shared_metrics_contract.py # hermes_cli/observability/shared_metrics_subscriber.py # scripts/smoke_nemo_relay_shared_metrics.py # tests/hermes_cli/test_relay_shared_metrics.py # tests/hermes_cli/test_relay_shared_metrics_runtime.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com> # Conflicts: # tests/tools/test_skills_hub.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Hitting ⌘1 (or cycling ⌃Tab onto the main tab) while Capabilities / Messaging / Artifacts covered the workspace looked dead: the workspace pane was already the zone's active tab behind the page, so fronting it changed nothing on screen. activateTreeTabSlot / cycleTreeTabInFocusedZone now return the activated pane id, and the keybind handlers route back to the loaded session (or the new-chat draft) when the landing pane is the workspace under a full page — the same rule openSession already applies.
…fyNative
Desktop plugins can toast in-app (host.notify) but have no sanctioned way to
reach the OS notification pipeline the app's own approval/turn alerts use, so
a plugin surfacing a genuinely notable background event (e.g. a discovery
plugin finding a match) stays invisible once the user steps away from Hermes.
Add a curated per-plugin door instead of exporting the raw dispatcher:
- ctx.notifyNative({ title, body?, silent? }) on PluginContext — attributed
to the plugin id, routed through dispatchNativeNotification so every
existing gate applies (master + per-kind prefs, post-connect baseline,
away-from-app gating, throttle).
- New 'plugin' native-notification kind with its own Settings ▸ Notifications
toggle (default on), so users silence plugins without losing app alerts.
- New optional `tag` discriminator on the notify payload keys the renderer
throttle and main-process cross-window dedupe per plugin, so two plugins
can't collapse each other's session-less notifications.
Consumer: the Index Network desktop plugin wants background opportunity
alerts; anything in ~/.hermes/desktop-plugins gets the same door.
…gets messageRenderWeight moves out of thread/list.tsx into lib/render-weight.ts. The DOM page budget already spends render cost rather than message count — the store window added next needs the same currency, and one weight function keeps the two layers from drifting apart. No behavior change.
…st (NousResearch#55191) An oversized session rebuilt an unbounded runtime repository on every store update and exhausted the renderer's V8 heap, crash-looping the window. The DOM budget in thread/list.tsx bounds what PAINTS, but every message was still normalized into the repository first, so a session only had to be heavy — not visible — to kill the renderer. selectTranscriptWindow keeps the tail that fits one render-weight page. Weight, not message count: measured against a real 1,175-session store, a 400-message cap disengages on 37 sessions that are heavy but short (one is 133 messages / 1.05MB) while firing on 92 long-but-light sessions that were never at risk. The cut aligns off branch-group boundaries. useRuntimeMessageRepository records a group's fork point the first time it sees the group, so a window starting mid-group would re-parent the surviving branches to whatever happened to precede them. Co-authored-by: HexLab <8422520+HexLab98@users.noreply.github.com>
…rom the store Show earlier spends the already-materialized DOM budget first and only asks the session store for another page once that is exhausted, so the click stays cheap and the store window stays as small as it can be. Paging has no ceiling: each expand grows the window by one budget page until the whole transcript is loaded. Branch persistence stays wired throughout — setMessages is never dropped, so switchToBranch and applyBranchVisibility keep working on a windowed session. Co-authored-by: HexLab <8422520+HexLab98@users.noreply.github.com>
Fold ctx.notifyNative into a ctx.os namespace so every way a plugin reaches outside the app window lives behind one attributed door instead of accreting one top-level ctx method per capability: - ctx.os.notify — the native-notification door from the previous commit, unchanged semantics (plugin kind pref, away-gating, per-plugin throttle). - ctx.os.openExternal / ctx.os.revealPath / ctx.os.writeClipboard — the existing window.hermesDesktop bridge capabilities, now sanctioned and result-shaped: each resolves false (never throws) when the bridge or member is missing, so a plugin branches on the result instead of sniffing the preload surface or crashing on an older shell. No new Electron surface: everything routes through bridge members the app already ships; the notification path keeps every existing gate.
…earch#22622) * fix(credential-pool): clear exhaustion state on key rotation When a user rotates an API key (e.g. via `hermes setup` after hitting a rate limit), _upsert_entry updates the access_token on the existing pool entry but preserves the stale last_status=exhausted from the old key. On the next session the pool finds the entry, sees it exhausted, and returns no usable credentials — even though the new key is valid. Fix: when access_token changes on an existing entry, reset last_status, last_error_code, last_error_reason, last_error_message, and last_error_reset_at. The exhaustion state belongs to the old key, not the new one. * chore: add pasevin@gmail.com to AUTHOR_MAP * fix: clear last_status_at on key rotation, remove unused pytest import Address review feedback from teknium1 on PR NousResearch#22622: - Add last_status_at=None to the reset block (matches all other token-sync reset paths in credential_pool.py) - Assert last_status_at is None in the regression test - Remove unused pytest import flagged by ruff + ty
…eave-pages ⌘1 / ⌃Tab return to the chat from a full-page view
…lay-client-dimensions feat(observability): add Relay client resource metrics
…lay-install-activation-metrics feat(observability): add Relay active install metrics
…ersedes NousResearch#47876, NousResearch#47921, NousResearch#24315) (NousResearch#79524) * fix(dashboard): add events-feed reconnect policy helpers Extract the reconnect arithmetic and close-code classification for the ChatSidebar /api/events socket into a pure module so both can be tested without a fake WebSocket or a mounted component. Two decisions live here rather than inline in the effect: - `shouldRetryEventsClose` — 1000 (normal) and 4401/4403 (auth) are terminal; everything else, including 1005/1006 from a killed gateway or a dropped network, is retryable. - `isEventsFeedMessage` — the sidebar's banner is shared with `info.credential_warning` and the JSON-RPC sidecar, so a reconnect may only clear a message the events feed wrote itself. Co-authored-by: Ishan Parihar <ishan@supreme-god.dev> Co-authored-by: Vyre <vyre@ishanparihar.com> Co-authored-by: eric-senyao <178080753+eric-senyao@users.noreply.github.com> * fix(dashboard): auto-reconnect the events WebSocket with backoff The chat sidebar's /api/events subscriber surfaced a static "disconnected" banner on a transient drop and never retried, so a gateway restart or a network blip left the feed dead until the user reloaded the page. The feed drives the live chat title (session.info) and dashboard.new_session_requested, both of which silently stopped working. Reconnect with exponential backoff (1s → 2s → 4s → … → 30s cap, 15 attempts then a terminal banner). Specifically: - One scheduling path. `close` always follows `error` for a failed socket, so scheduling from both — as the superseded PRs did — queues two timers and leaks the one that is no longer tracked for cleanup. `scheduleReconnect` returns early when a retry is already pending. - The auth ticket is re-minted per attempt via `buildWsUrl`; tickets are single-use with a short TTL, so replaying the first URL would 4401 on the second attempt. - A superseded socket's late close cannot schedule a retry on top of its replacement (`isCurrent` generation check). - A successful open resets the backoff and clears only the events feed's own banner, leaving a credential warning or sidecar error visible. - The pending timer is cleared on unmount, not merely neutered by the `unmounting` flag. Also retires two strings the tools box left behind when it was removed in 47fccc0 (NousResearch#51737): the banner no longer promises "tool calls may not appear" and the button reads "reconnect events feed". Co-authored-by: Ishan Parihar <ishan@supreme-god.dev> Co-authored-by: Vyre <vyre@ishanparihar.com> Co-authored-by: eric-senyao <178080753+eric-senyao@users.noreply.github.com> * test(dashboard): cover the events-feed reconnect bug class Fake-timer coverage for the behaviors the superseded PRs changed without tests. Each case was mutation-checked — reverting the corresponding guard in ChatSidebar.tsx makes exactly that test fail: - transient close reconnects, and backoff grows 1s → 2s → 4s - error + close on one socket schedules ONE retry, not two - a successful open resets the backoff to 1s - 4401/4403 and a normal 1000 close never retry - the attempt cap stops the loop instead of retrying forever - reconnect clears the feed's own banner but not a credential warning - unmount clears the pending timer (asserted via `vi.getTimerCount()`, since the `unmounting` flag alone hides a leaked timer) Co-authored-by: Ishan Parihar <ishan@supreme-god.dev> Co-authored-by: Vyre <vyre@ishanparihar.com> Co-authored-by: eric-senyao <178080753+eric-senyao@users.noreply.github.com> * fix(dashboard): stop the events feed overwriting a foreign banner Review catch: `clearEventsBanner` guarded the shared banner but `surface` did not, so the guard was only half applied. A sidecar error or `credential_warning` already on screen when the feed dropped was replaced by "events feed disconnected" — and lost for good, since `error` is that message's only home and the sidecar does not re-emit. `surface` now writes only over an empty banner or one of the feed's own messages. Declining to write does not affect the retry itself; the reconnect still runs on schedule, it just stays silent while a more important message holds the banner. Both directions are covered: a foreign banner survives a drop, and the reconnect still fires while suppressed. --------- Co-authored-by: Ishan Parihar <ishan@supreme-god.dev> Co-authored-by: Vyre <vyre@ishanparihar.com> Co-authored-by: eric-senyao <178080753+eric-senyao@users.noreply.github.com>
…beval) Ships the hard A/B evaluation used for the August 2026 core-toolset performance batch (NousResearch#77056) as a reusable harness: 9 error-inducing trap tasks derived from measured production waste classes, two-arm PYTHONPATH-only comparison, ATOF-trace-based scoring, resume-safe batteries. Hardened from the original one-off: paths de-hardcoded (ABEVAL_ROOT / ABEVAL_HOME), encoding= on all file IO, startup crashes retry on resume instead of polluting cells, post-hoc grading fix for err_inline_script baked in. Live-smoked end to end (baseline arm, qwen3-coder-30b, err_multi_dir: exit 0, correct on-disk verification, resume record written).
archive_and_compact's new model_config_patch block was the third near-identical SELECT -> tolerant-parse -> merge -> UPDATE copy in hermes_state.py (update_session_runtime_lock and set_session_yolo carry the other two). Extract _merge_model_config_json(conn, sid, patch, on_missing=...) and route all three through it, preserving each caller's missing-row policy (flag setters skip, archive raises). Also adds the two small accessors the compressor needs: - patch_session_model_config(): standalone atomic merge for callers that must update model_config without rewriting the transcript - get_session_model_config_value(): tolerant single-key read Follow-up to the salvaged NousResearch#79286 commit, per the repo's extend-don't-duplicate rule.
…t no-op for incapable stores Three review follow-ups on the salvaged NousResearch#79286 commit: - update_model() zeroed the in-memory prune runway but left the durable model_config copy stale, breaking the method's own durable-sync discipline (the strike reset three lines above keeps its durable copy in sync). A restart after a model switch resurrected a runway computed under the old model's trigger sizes. New _clear_durable_proactive_prune_rearm() removes the persisted key via patch_session_model_config() without touching the transcript. - The archive_and_compact capability check ran AFTER the expensive 3-pass prune scan, so a duck-typed session store lacking the method paid the full scan on every eligible iteration forever with pruning permanently no-opping. Hoist it above the scan (all in-tree stores pass a real SessionDB; this only affects third-party stores). - _load_proactive_prune_rearm_tokens now uses the shared get_session_model_config_value() accessor instead of inlining a 5th copy of the model_config JSON parse, matching its sibling loaders' typed-accessor pattern. Also documents why the rotation-publish-failure branch restores only the runway field rather than the full attempt snapshot. Tests: model-switch durable clear, patch_session_model_config merge/delete/no-op, and a guard proving incapable stores skip the scan.
- fetch_models(): accept base_url kwarg (interface grew on main since May) - runtime_provider: config-driven loopback base_url now reaches the local no-auth placeholder before the usable-secret gate (added on main in the interim, would otherwise AuthError on keyless local setups) - test: fetch is now called with base_url by the generic live-fetch path
…ning-effort clamp for Actual - config_defaults: ACTUAL_API_KEY / ACTUAL_BASE_URL entries (setup wizard + hermes tools) - codex transport: clamp xhigh->high, ultra->max for provider=actual (SGLang/vLLM backends reject the wider values with a wrapped HTTP 400) - chat_completion_helpers: thread provider into Responses build_kwargs - tests: transport clamp + config-driven local no-auth regression
- optional-skills/devops/actual-setup: field-tested setup skill contributed by shl0ms, updated for the first-class 'actual' provider (the original targeted a custom-provider config that now collides with the built-in name) - docs: providers.md section + tables, environment-variables.md, quickstart.md - tests/skills: frontmatter + first-class-provider conformance checks
Record an unsplit stream ledger for multi-message deliveries and refuse legacy trust when split delivery left no payload, so Telegram group sessions no longer suppress a complete reply after an early/partial finalize (NousResearch#78541).
Add unit and GatewayRunner boundary coverage for the NousResearch#78541 shape where final_content_delivered is set via split delivery with no recorded payload.
…uplicate or still swallow The salvaged fix changed only the gateway's verdict: a payload-less multi-message split stopped inheriting legacy trust. But six code paths set _turn_split_delivery, and only one of them was taught to record a payload, so the remaining five swapped the swallow for the opposite defect. Fix the producers instead of only distrusting them at the boundary: - _send_or_edit failed-final-edit branch: record the visible payload on split turns too. It deliberately skipped recording, which now reads as a mismatch and re-sends an answer already on screen -- reintroducing the duplicate NousResearch#45517 fixed (NousResearch#36965 / NousResearch#25349). - _send_fallback_final (x2) and _send_empty_fallback_final: route through _record_turn_final_payload instead of assigning _delivered_final_text directly. On a split turn their final_text is only the trailing chunk, so a fully delivered heads+tail reply recorded a tail-only payload and was re-sent in full. - _try_fresh_final: refuse the fresh-final route once a head chunk is sealed. It replaces every tracked preview with one message, which only holds the whole answer on a single-message turn. After a split it deleted the sealed heads while sending just the tail, so the complete reply was still lost -- on Telegram, the default finalize route and the shape NousResearch#78541 reports. - Set _turn_split_delivery at seal time rather than after the tail send, so the tail's own finalize sees the split state. The sibling overflow path already did this; the divergence is what let fresh-final delete the heads. - run.py stale-finalize reconciliation: skip the in-place edit on a split delivery. message_id is only the LAST chunk there, so editing it with the complete response repeated every sealed head's text inside the tail message. Fall through to the normal final send. Also drop a dead `or "".join(chunks)` fallback (all growth funnels through _append_accumulated, so the ledger is never empty at that call site, and joined chunks carry injected fence markers that could never match final_response), and document that _record_turn_final_payload intentionally ignores its argument on split turns. Tests: four end-to-end cases driving the real overflow-split loop instead of hand-setting private flags -- complete split still suppresses (no duplicate), split missing a tail does not suppress, fresh-final keeps sealed heads, and a flood-controlled final edit after a split stays suppressed. Each was mutation-checked: reverting any individual fix turns its test red. The pre-existing gateway-boundary test asserted the recovery *route* (the reconcile edit) rather than the guarantee. Relaxed to the real contract: either _run_agent puts the complete text on the wire, or it declines to claim delivery so the caller's normal final send does. Co-authored-by: HexLab98 <liruixinch@outlook.com>
…ty-fallback recovery Follow-up to NousResearch#79669. That PR routed the three fallback recorder sites through _record_turn_final_payload so a split turn would record the unsplit ledger instead of a tail-only payload. For two of them that is right. For _send_empty_fallback_final it is wrong, and it reintroduces the NousResearch#78541 swallow at the one site that was supposed to be fixed. _send_empty_fallback_final is a *replacement* recovery: it sends the completed text as a fresh message and deletes every tracked segment preview -- which on an overflow split includes the sealed head chunks. After it runs, the only thing on screen is the message it just sent. Recording the ledger there claims delivery for text the same function just removed, so delivered_final_matches() returns True, the gateway suppresses its own send, and the user is left with a fraction of the answer. Observed with a probe driving the real run() loop (543-char reply, 475-char head sealed then deleted, 67-char tail committed): before this fix recorded=543 matches=True -> suppressed, 67/543 on screen after this fix recorded=67 matches=False -> gateway sends the full answer Record final_text verbatim here instead. The sibling site in _send_fallback_final keeps the recorder: its delete is gated on `continuation == final_text` and targets only the single active partial, never the sealed heads, so the ledger correctly describes what survives. The distinction is whether a recovery ADDS to what is on screen or REPLACES it. Additive paths may record the ledger; replacing paths must record only what they leave behind. _try_fresh_final is the same shape and NousResearch#79669 handled it by refusing the route on split turns. Test drives the real seal-then-delete sequence and asserts the mismatch, so the gateway is required to re-send. Mutation-checked: restoring the recorder call turns it red.
…identity
A kanban worker that fires a cron job in-process no longer leaks its task
identity into the cron agent.
The worker is a normal `hermes chat -q` CLI agent whose default toolset
includes `cronjob`, running with HERMES_KANBAN_TASK legitimately set in its
own environment. `cronjob(action="run")` calls run_one_job() -> run_job()
in that same process, so the cron AIAgent was misidentified as that worker:
kanban toolset force-added, kanban-worker protocol injected into its system
prompt, and kanban_complete defaulting task_id to $HERMES_KANBAN_TASK --
letting an unrelated cron job close the worker's task and overwrite real
results.
Fixed with a ContextVar (`non_dispatcher_owned_context`), not by clearing
os.environ. The env is process-global and shared with three concurrent
readers that all need the real values:
* the worker's own claim heartbeat -- run_agent._touch_activity ->
heartbeat_current_worker_from_env reads TASK/CLAIM_LOCK/RUN_ID, and the
cron-run heartbeat thread drives it every 10s. Clearing them silently
no-ops the heartbeat, so after DEFAULT_CLAIM_TTL_SECONDS (15 min) the
dispatcher reclaims a task whose worker is still alive and re-dispatches
it -- the same duplicate-work failure from the other direction.
* the gateway's kanban watchers, which do their own HERMES_KANBAN_BOARD
save/restore around a slow decompose_task() LLM call.
* concurrent cron jobs, which take a *shared* read lock
(_terminal_cwd_lock.acquire_read) and so interleave: job A clears, job B
snapshots empty, A restores, B clears and its restore no-ops -- the
worker's identity is destroyed permanently.
`is_dispatcher_owned_worker_context()` is now the single predicate every
HERMES_KANBAN_* identity gate consults before trusting those vars. It also
closes a pre-existing gap in agent/skill_utils.py, which read the vars
without consulting the delegate_task ContextVar at all; the `kanban` verdict
additionally bypasses _ENV_DETECT_CACHE, since a context-dependent answer
must not be memoized process-wide.
HERMES_KANBAN_BOARD/DB/WORKSPACES_ROOT are left untouched, so the NousResearch#20074
board pin and the dispatcher's path overrides keep working.
Tests: 18 new, including thread-isolation, concurrent-cron-jobs, and an AST
invariant over _default_spawn that fails if the dispatcher gains a var that
is neither identity-gated nor explicitly classified behaviour-only. All six
mutations are caught, including one that reintroduces the os.environ clear.
tests/cron/ + kanban suites 440 passed; model_tools/skill_utils/boards 63
passed; ruff clean.
Reported and diagnosed by Geoff Friesen (NousResearch#78961), who identified the symptom
and the exact gating mechanism.
Co-authored-by: Geoff Friesen <gfriesen1@users.noreply.github.com>
The AST invariant only matched `env["HERMES_KANBAN_X"] = ...` subscript
assignments, so a future dispatcher var added via `env.update({...})`,
`env.setdefault(...)`, or an annotated subscript would have slipped past
the guard and leaked into cron sessions unnoticed.
None of those shapes exist in _default_spawn today; this is about the
guard staying trustworthy as that function evolves.
Verified by injecting an unregistered var into _default_spawn one shape at
a time and requiring the guard to fail: subscript assign, annotated
assign, update(dict literal), setdefault(literal), and update(kwarg) are
all detected. Source restored byte-identical after probing.
tests/cron/ 410 passed; ruff clean.
…pty-fallback-deleted-heads fix(gateway): don't claim deleted head chunks as delivered in the empty-fallback recovery
_begin_in_order parks each concurrent tool worker on a timeout-less Condition.wait_for until every earlier-ordered tool has advanced through its dispatch. If one tool wedges during dispatch (observed in production: a stuck skill_view; also reproducible via any blocking authorization), three failures compound: every later-ordered worker is starved and never starts; the batch deadline then falsely reports those never-started tools as "timed out" (sub-second read_file/search_files calls get blamed while having done zero work, and the model reasons against that false failure info); and after the batch is abandoned the parked workers leak forever — f.cancel() cannot cancel running threads, the per-thread interrupt flag is never polled inside wait_for, and nothing notifies the condition again. Confirmed with a faulthandler all-threads dump taken after batch abandonment showing workers still parked at the gate. Bound the wait at 120s; on expiry, log a warning and proceed out of order (worst case: interleaved approval prompts — strictly better than permanent starvation). The >= predicate lets one worker's timeout-jump release every skipped worker immediately, and max() keeps the counter monotonic for out-of-order advancement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ort abandoned workers
Follow-up to the salvaged start-order gate bound. Two gaps remained, both
reachable through the same knob.
1. The gate bound ignored the batch deadline it sits under. With
HERMES_CONCURRENT_TOOL_TIMEOUT_S below 120s the deadline fired first, so
the parked tools were still reported as "timed out" without ever running --
the exact bug the bound exists to fix. The gate now clamps to
min(120s, batch_timeout / 2), matching the sibling constant's documented
habit of relating the two timeouts.
2. A gate-parked worker released purely by its own timeout could wake up after
the batch was abandoned and dispatch its tool anyway: wasted work whose
result nobody reads, a duplicate post_tool_call for a tool_call_id the turn
already closed as timeout, and agent._current_tool left pointing at a dead
tool for the rest of the session (the main thread's reset already ran).
Abandonment is now a first-class wakeup: both abandon sites set an event and
notify the condition, and a released worker raises _BatchAbandoned instead
of dispatching. Parked threads are reclaimed in milliseconds rather than one
full gate timeout plus a tool runtime.
Also names the tool in the gate-timeout warning. The closure's function_name
binds the last-parsed tool, so logging it directly would have printed the wrong
name; it is threaded through _begin_in_order instead.
Measured, 3-tool batch with the first tool wedged during dispatch:
main PR as-is with this commit
dispatched in batch 0 0 tool_b, tool_c
dispatched after return 0 2 (ghost) 0
_current_tool leaked no "tool_b" no
Adds tests/run_agent/test_start_order_gate.py (3 tests). Mutation-checked
against the parent commit: the starvation guard passes there (it binds the
salvaged fix), while the deadline-clamp and abandonment guards both fail,
reproducing the ghost dispatch as
"tool(s) dispatched after the batch was abandoned: [tool_a, tool_b]".
Adds contributors/emails/sylbae@users.noreply.github.com -> sylbae so check-attribution passes for the salvaged commit in this PR. Bare user@users.noreply.github.com addresses are not auto-skipped by the CI check (only the numeric NNN+user@ form is), so the mapping file is required.
_prune_filter_where documents title_like / model_like / branch_like as "case-insensitive substring matches", and the CLI confirmation renders them as "title contains 'X'". They were bound straight into a bare LIKE, so `_` matched any single character and `%` any run. The builder backs prune_sessions(), which deletes session rows and their on-disk transcripts, so the over-match is unrecoverable: pruning title_like="user_auth" also destroys "user-auth", "userXauth" and "user auth". `_` is not exotic here -- git branch names and session titles carry it routinely. Escape the operator's needle and add ESCAPE '\' to the three clauses, the same convention the rest of this file already uses for LIKE queries. Match direction is unchanged for needles without wildcards. Left alone: _cwd_prefix_clause has the same unescaped shape but is shared by four call sites beyond prune, so it is a separate change.
_cwd_prefix_clause builds "cwd is this directory or under it" for session
listing, workspace resume and prune/archive. The two LIKE arms bound the
raw prefix, so `_` and `%` acted as wildcards on a value that is a path:
cwd_prefix="/home/me/my_project"
main -> ['sibling', 'target'] # /home/me/myXproject/src matched too
fix -> ['target']
`_` matches any single character, so a same-length sibling directory with
children falls inside the pattern. prune_sessions() deletes the rows it
matches (and their on-disk transcripts), so an unrelated project's history
goes with it.
Escape the needle and pair both arms with ESCAPE, the convention the rest
of this file already uses; the literal separator backslash in the Windows
pattern is escaped for the same reason. The `=` arm is an exact compare and
keeps the raw prefix, so directory-and-children matching is unchanged.
Follow-up to the *_like filters in NousResearch#78681, kept separate because this helper
is shared by four call sites beyond prune.
Follow-up on the salvage of NousResearch#78681 + NousResearch#78927: the second fix inlined the exact body of the _escape_like helper the first fix introduced ten lines above. Call the helper instead so there is one copy of the escaping rule.
…clause The quadruple-backslash pattern arm is the trickiest byte sequence in the fix and had no direct coverage — 'simplifying' it to a double backslash would break Windows child matching with every test still green. Mutation checked: weakening the arm fails this test.
readZoomState() collapsed "file never existed" and "read failed for
any other reason" (corrupt JSON, transient I/O error, disk
contention) into the same null. restorePersistedZoomLevel() — which
runs on every window show/restore/resized/moved/did-finish-load —
treated any null as "never configured", fell back to the 90% shipped
default, and persisted that 90% back into zoom-state.json, silently
overwriting a real saved zoom (e.g. 150%).
readZoomState() now returns {level, missing}, using the error code
to distinguish ENOENT (genuinely never configured) from any other
read failure. Only the true first-run path may persist a fallback;
a transient read failure now applies the default in-memory for that
one call without touching the on-disk value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The desktop app's zoom setting (e.g. 150%) was randomly resetting to the 90% shipped default.
Root cause:
readZoomState()inapps/desktop/electron/main.tscollapsed two distinct cases into the samenull:restorePersistedZoomLevel()runs on every windowshow/restore/resized/moved/did-finish-load. On any transient read failure (case 2), it fell back to the 90% default and persisted that fallback intozoom-state.json, permanently overwriting the user's saved setting.Fix
readZoomState()now returns{ level, missing }, distinguishingENOENT(truly never configured) from any other read failure.restorePersistedZoomLevel()only writes a fallback to disk on true first-run (missing: true); a transient read failure applies the default in-memory for that one call and leaves the on-disk value untouched.Test plan
npx tsc --noEmit -p .cleannpx vitest run electron/zoom.test.ts— 17/17 passzoom-state.jsonsurvives window events without being clobbered