Skip to content

fix(hermes-bots): stop New Cronjob dialog crash when the owner is a roster object - #93572

Closed
zhangfei1231231-sketch wants to merge 1 commit into
NousResearch:mainfrom
zhangfei1231231-sketch:fix/routines-dialog-displayname-crash
Closed

fix(hermes-bots): stop New Cronjob dialog crash when the owner is a roster object#93572
zhangfei1231231-sketch wants to merge 1 commit into
NousResearch:mainfrom
zhangfei1231231-sketch:fix/routines-dialog-displayname-crash

Conversation

@zhangfei1231231-sketch

Copy link
Copy Markdown
Contributor

Summary

Clicking "+ New Cronjob" in the Cronjobs pane crashes the whole routines pane into its error boundary:

[error-boundary:contrib:hermes-bots:routines]
TypeError: (e.name || "").trim is not a function

The bot prop handed to CreateRoutineDialog is a roster-owner object: RoutinesPane.openCreate() stores the resolved owner from resolveRoutineOwner(), which always returns an object. The "Send results to" option label then wraps it unconditionally:

{ id: 'bot-chat', label: `${displayName({ name: bot }, $botMeta.get()[bot])}...` }

producing { name: <object> }, so displayName() calls (bot.name || '').trim() on an object and throws mid-render. The dialog header two lines above already normalizes correctly (typeof bot === 'string' ? { name: bot } : bot); this call site does not.

Deterministic: any user opening the dialog with a valid roster owner repros it.

Fix

  • Normalize at the call site, matching the header's existing pattern (and use botRosterMeta(bot, ...) so meta lookup works for object owners too).
  • Defensive guard in displayName(): a non-string bot.name degrades to '' instead of a render-time TypeError.

Related

This line came in with the same #90006 merge wave that produced the activeBotRoute() regression (#92830 / #92878 / fixes #92842, #92811); it is a residual from that refactor that survived the cleanup in #93125.

Testing

  • All hermes-bots test files pass locally; the 2 pre-existing failures in routine-prompt.test.mjs are identical on an unmodified checkout (verified via git stash) and untouched by this patch.
  • Manually verified: dialog renders, creates jobs, and the renderer error no longer appears in desktop logs.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) labels Aug 24, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Overall: the right instinct — stop the crash at the New Cronjob dialog's bot-chat label, plus a defensive normalization inside displayName. But the fixed call site reintroduces a subtler version of the same string/object confusion in its second argument, and the defensive branch has a small coercion hole.

  1. apps/desktop/src/plugins/hermes-bots/plugin.js:10879 — the new meta argument is botRosterMeta(bot, $botMeta.get()) with the original bot. Per botRosterMeta at plugin.js:3368, lookup is metaByName?.[bot?.name]. When bot is a string (the very case the first argument now guards with typeof bot === 'string'), bot?.name is undefined, so the meta lookup misses and displayName never sees meta?.title (checked first at plugin.js:3702). The old code keyed meta by the raw string ($botMeta.get()[bot]), which worked for string owners. Net effect: for string-typed bot props the label silently drops the user's configured title and falls back to the derived name. Hoist the normalization once, e.g. const botObj = typeof bot === 'string' ? { name: bot } : bot, then displayName(botObj, botRosterMeta(botObj, $botMeta.get())).

  2. apps/desktop/src/plugins/hermes-bots/plugin.js:5672 — the coercion only handles object-typed names: typeof bot.name === 'object' && bot.name !== null ? String(bot.name?.name ?? '') : ''. Any other non-string (number, boolean) degrades to '' instead of String(bot.name), which the comment claims should be the fallback ("degrade to ''" is stated, but a numeric name rendered as empty is just a quieter bug). Suggest typeof bot.name === 'object' ? String(bot.name?.name ?? '') : String(bot.name ?? '') — this also makes the explicit !== null check redundant (optional chaining covers it) and avoids String({...})"[object Object]" when bot.name.name is itself an object (guard a depth of one: only take .name if it's a string).

  3. ScopedisplayName is hot-rendered for every roster row; the spread at plugin.js:5672 reallocates the bot identity on each call for affected rows. Negligible here since it only fires on malformed input, but worth a comment that the branch is a cold error path.

Minor: a small vitest case around CreateRoutineDialog with both a string owner and a roster-object owner would pin down both sides of finding 1 and prevent the "fix one arm, break the other" pattern from repeating.

teknium1 pushed a commit that referenced this pull request Aug 25, 2026
…oster object

CreateRoutineDialog receives routineCreateTarget() output, which is an
owner OBJECT for roster-scoped bots; wrapping it in {name: bot} rendered
'[object Object]' and broke the meta lookup keyed by object. Resolve the
label through the object-aware botRosterMeta() path instead.

(Salvaged from #93572; the defensive coercion inside displayName was
dropped in favor of fixing the call site only.)
teknium1 added a commit that referenced this pull request Aug 25, 2026
teknium1 pushed a commit that referenced this pull request Aug 25, 2026
…oster object

CreateRoutineDialog receives routineCreateTarget() output, which is an
owner OBJECT for roster-scoped bots; wrapping it in {name: bot} rendered
'[object Object]' and broke the meta lookup keyed by object. Resolve the
label through the object-aware botRosterMeta() path instead.

(Salvaged from #93572; the defensive coercion inside displayName was
dropped in favor of fixing the call site only.)
teknium1 added a commit that referenced this pull request Aug 25, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #95018 (rebase-merge) — your call-site fix is on main as-authored: 9033545. Thanks @zhangfei1231231-sketch!

Note on scope: we took the CreateRoutineDialog call-site half and dropped the defensive coercion inside displayName() — the call site was the bug, and masking non-string names at the shared renderer would hide future mis-callers. A regression test pinning the object-aware call shape was added on top. Live-verified before merge: the dialog now opens cleanly for roster-scoped bots ("A recurring task Testbot runs on a schedule") where origin/main crashed the pane.

@teknium1 teknium1 closed this Aug 25, 2026
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
…oster object

CreateRoutineDialog receives routineCreateTarget() output, which is an
owner OBJECT for roster-scoped bots; wrapping it in {name: bot} rendered
'[object Object]' and broke the meta lookup keyed by object. Resolve the
label through the object-aware botRosterMeta() path instead.

(Salvaged from NousResearch#93572; the defensive coercion inside displayName was
dropped in favor of fixing the call site only.)
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants