Skip to content

Answer an unreadable token identifier instead of throwing on it - #123

Merged
portdeveloper merged 1 commit into
portdeveloper:mainfrom
BeeHiveTeam:fix/token-id-unreadable
Oct 9, 2026
Merged

portdeveloper merged 1 commit into
portdeveloper:mainfrom
BeeHiveTeam:fix/token-id-unreadable

Conversation

@BeeHiveTeam

@BeeHiveTeam BeeHiveTeam commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #122.

isNativeToken coerced with a bare String(), which is not total: it throws for a value whose toString is not callable, and {"toString": null} is ordinary JSON a model can emit. That call runs before the resolveToken try/catch, so the throw left runAction entirely.

Measured on 9871f73, with the controls in the last two rows:

get_token_balance with before after
token: {toString: null} throws TypeError: Cannot convert object to primitive value Refused: invalid token "<unprintable>": Cannot convert object to primitive value
symbol: {toString: null} same throw same refusal
tokenAddress: {toString: null} same throw same refusal
through dispatchToolCall same throw same refusal
token: "NOT_A_TOKEN" (control) Unknown token "NOT_A_TOKEN"… unchanged
token: "MON" (control) native coin unchanged

The refusal keeps the converter's own words after the colon. That shape comes from the resolveToken try/catch ca31f86 added, not from this change — and it is where the Refused: prefix and the recognised status come from, since resolveToken throws on this value too. Worth saying plainly so the table is not read as a cleaner message than it is.

isRefusal() now reads true for all four, where before it read false for a rejection — so a scripted caller keeps its failure status instead of exiting 0 on it. The refusal arrives before any wallet or RPC call: with no wallet configured, anything that reached one would say Wallet not initialized, and the new tests assert that it does not.

Where the fix went, and why there

In the predicate, not at the call site. isNativeToken is called from three places with model-supplied values, so a predicate that can throw is a hazard at each of them; normalizing inside get_token_balance alone would leave the other two able to throw. A value String() cannot read is certainly not the native symbol, so false is the answer — it only has to be answered rather than thrown.

safeEcho already takes this shape of value for the same reason and returns <unprintable> (addressBook.mjs), so the refusal message needs no coercion of its own.

Readable identifiers keep their old answer exactly: the coercion, the trim and the comparison are unchanged, and the missing-token message for an unrecognised symbol is untouched, as the issue asks.

Coverage

596 tests, up from 589. Seven new: the three aliases, the native dispatcher, the refusal arriving before any wallet or RPC call, and the controls.

One of them exists because the mutation pass found it. Removing the trim or the toUpperCase from this predicate changes what counts as the native coin and reddened nothing on main. That property predates this change, but the change rewrites the predicate that holds it and the issue asks to preserve MON handling — so "MON", "mon", " MON ", "Mon" and a tab-padded form are now pinned, along with "MONX", "XMON" and "" not being native.

Mutations

Seven mutations, each reverted on its own with the full suite run each time, plus a no-op control:

reverted tests red
control — the local renamed, nothing else 0 (596 pass)
the fix reverted to the one-liner, so the coercion throws again 5
an unreadable value treated as the native coin 5
the trim dropped 1
the toUpperCase dropped 1
the catch continuing with "" instead of returning false 0 — equivalent
?? "" dropped 0 — equivalent

The two zeros are equivalent rather than uncovered, and both are provable rather than argued: SYMBOL() is MON, so "".trim().toUpperCase() cannot match it and continuing with "" gives the same answer as returning false; and String(null) is "null", which cannot match it either, so dropping ?? "" changes no outcome.

Two things measured and deliberately not touched

This closes the four execution paths the issue names. Two neighbours carry the same coercion and are left alone, because "do not broaden this into a rewrite of other actions" reads to me as leaving them. Reported rather than fixed, and I would take either as a separate issue.

The parse path still loses the action. After this change normalizeParsedAction no longer throws, but hasRequiredArgs does, at its own String(action[candidate]).trim(). parseAction catches it in the "try the next JSON object" handler and returns {action: "none"}:

{"action":"get_balance","token":{"toString":null}}        -> {"action":"none"}
{"action":"get_token_balance","token":{"toString":null}}  -> {"action":"none"}
{"action":"get_balance","token":"USDC"}  (control)        -> {"action":"get_token_balance","token":"USDC"}

An operator who asks for a balance has the request silently dropped and gets a chat reply instead. I checked that this is unchanged by this PR: the parser's output is byte-identical before and after, including inputs with two JSON objects in one message and the text fallback.

The send path throws the same way. prepareTokenSend coerces the same field, and its call site is not wrapped:

prepareTokenSend({action:"send_token", token:{toString:null}, …})
  -> throws TypeError: Cannot convert object to primitive value

Reachable through native tools only: nativeToolLoop builds handleAction({ action: name, ...args }) straight from the tool call (:76), so hasRequiredArgs never gates it, while the v0 path is stopped by the parse behaviour above. The throw is caught at the top of the loop, printed as Error: …, and hadFailure is still set in a scripted run — so the session survives and the exit status stays correct. What is lost is the message quality, which is this issue's class on the write side.

Gates

npm run build clean; suite run three times at 596/596. Two files, two hunks, +103 −1; longest added line 93 characters.

Environment

  • branch: main @ 9871f73
  • OS: Linux x64, node v22
  • Gas mode: n/a — the repro and the new tests run through runAction and dispatchToolCall with no wallet and no network

`isNativeToken` coerced with a bare `String()`, which is not total: it throws for a
value whose `toString` is not callable, and `{"toString": null}` is ordinary JSON a
model can emit. The call in `get_token_balance` runs before the `resolveToken`
try/catch, so the throw left `runAction` entirely — `handleAction` printed `error: …`
rather than a refusal, `isRefusal()` read false for what is a rejection, and a scripted
run exited 0 on a rejected call. Every alias of the field reaches that same line, so
`token`, `symbol` and `tokenAddress` all did it, and so did the native dispatcher.

The fix is in the predicate rather than at the call site, which is a choice worth
stating. `isNativeToken` is called from three places with model-supplied values, so a
predicate that can throw is a hazard at each of them; normalizing in
`get_token_balance` alone would leave the other two able to throw. A value `String()`
cannot read is certainly not the native symbol, so false is the answer — it only has to
be answered rather than thrown.

`safeEcho` already takes this shape of value for the same reason and returns
`<unprintable>`, so the refusal reads `Refused: invalid token "<unprintable>"` without a
second coercion of its own.

Readable identifiers keep their old answer exactly: the coercion, the trim and the
comparison are unchanged, and the missing-token message for an unrecognised symbol is
untouched.

596 tests, up from 589. The seven new ones cover the three aliases, the native
dispatcher, the refusal arriving before any wallet or RPC call, and the controls — an
unknown symbol still answering with the missing-token message rather than a refusal.

One of them exists because the mutation pass found it: removing the trim or the
toUpperCase from this predicate changed what counts as the native coin and reddened
nothing. That property predates this change, but the change rewrites the predicate that
holds it, so " MON ", "mon" and a tab-padded form are now pinned.

@portdeveloper portdeveloper left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for covering the aliases and preserving native-token recognition

@portdeveloper
portdeveloper merged commit ca928ac into portdeveloper:main Oct 9, 2026
3 checks passed
portdeveloper pushed a commit that referenced this pull request Oct 13, 2026
get_token_balance answered a token it could not resolve with the bare
"Unknown token ..." text, so isRefusal() was false and a scripted run
finished with exit 0 after a balance read that never happened. The send
path already prefixes the same explanation with "Refused:". The balance
read now does too, keeping the known-symbol and contract-address hint.

Tests cover "NOT_A_TOKEN", "", 0 and false through runAction and native
dispatch, and a real dist/cli.mjs run with a shimmed model that reads an
unknown token and then replies normally still exits 1. The #123 control
for the unknown symbol and the empty-catalog message now expect the
refusal; MON, known-token, contract-address and #123's unreadable-value
handling are unchanged.

Closes #124
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refuse unreadable get_token_balance identifiers before native-token detection

2 participants