KeeperHub / KeeperHub/keeperhub
feat(protocols): protocol read actions advertise output fields that do not exist at runtime
- Dominant language
- TypeScript
- Stars
- 24
- Forks
- 93
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 266
Description
### Before filing
- [x] I searched open and closed issues for this proposal.
- [x] I checked the docs and the current behaviour on `staging`.
- [x] This is one change, not several.
### Reason: what happens, and what told me to expect otherwise
A protocol read action advertises output fields that do not exist in its own runtime result, so a template built from the builder's own suggestion resolves to nothing.
`lido/get-wsteth-balance` on `staging` at `f8c8f18c7`:
```
registry declares : ["balance"]
ABI outputs : [{"name":"","type":"uint256"}]
runtime result : "12345"
advertised fields : ["balance","success","error"]
```
The builder offers `balance`. The runtime result is a bare string. `{{Node.balance}}` resolves to undefined. So does `{{Node.result.balance}}`, because there is no `balance` key at any depth. The only reachable path is `{{Node.result}}`.
Two separate faults produce this, both at `lib/protocol-registry.ts:494`:
```ts
outputs.push({ field: output.name, description: output.label });
```
1. **The field is emitted unprefixed.** `output.name` is `balance`, but every runtime value lives under `result`. Even when the name is correct the path is wrong.
2. **The field is emitted whether or not the ABI names the output.** `structureAbiOutputs` (`plugins/web3/steps/structure-abi-result.ts:92-98`) wraps a single output in `{ [name]: value }` *only when the ABI output carries a name*. The `outputs:` override in a protocol definition renames the model layer and never reaches the decoder, so for an unnamed ABI output the declared name describes a key that is never created.
**What told me to expect otherwise.** Two things in this repo, both specific:
`lib/workflow/editor/action-output-fields.ts:82-108` solves exactly this problem for raw web3 nodes and gets both halves right. It prefixes `result.`, then on an unnamed single output it deliberately does not invent a name:
```ts
} else {
// Unnamed single output: a tuple's components surface directly under
// result; a scalar is just `result` (already added).
appendTupleComponentPaths(outputFields, "result", output, 0);
}
```
And the comment directly above the protocol-path line, `lib/protocol-registry.ts:488`, rules out this failure mode for writes on precisely these grounds:
> KEEP-296: only reads surface `action.outputs` as UI template suggestions. Write actions still have ABI-derived outputs at the model layer, but `writeContractCore` returns `result: undefined`, so surfacing them would create template suggestions that resolve to undefined at runtime.
Writes were fixed because they would advertise fields resolving to undefined. Reads do the same thing and were not.
### Reason: what it costs
204 read-action output fields across 19 of the 26 registered protocols are advertised but absent at runtime.
| protocol | fields | protocol | fields | protocol | fields |
|---|---|---|---|---|---|
| sky | 31 | chainlink | 16 | lido | 7 |
| yearn | 27 | spark | 14 | cowswap | 6 |
| chronicle | 26 | pendle | 11 | curve | 6 |
| ethena | 17 | compound | 9 | aave-v4 | 5 |
| | | layerzero | 9 | safe | 5 |
| | | aerodrome | 7 | superfluid | 4 |
| | | | | uniswap 2, frax-ether-v2 1, wrapped 1 |
Reproduction, from a clean checkout after `pnpm discover-plugins`:
```bash
pnpm tsx -e '
import "./protocols/index";
import { getRegisteredProtocols } from "@/lib/protocol-registry";
let phantom = 0; const byProto = {};
for (const p of getRegisteredProtocols()) for (const a of p.actions) {
if (a.type !== "read") continue;
const c = p.contracts[a.contract]; if (!c?.abi) continue;
let abi; try { abi = JSON.parse(c.abi); } catch { continue; }
const fn = abi.find(e => e.type === "function" && e.name === a.function);
if (!fn) continue;
const outs = fn.outputs || [];
let runtimeKeys;
if (outs.length === 0) runtimeKeys = [];
else if (outs.length === 1) {
const n = (outs[0].name || "").trim();
runtimeKeys = n ? [n]
: ((outs[0].type || "").startsWith("tuple")
? (outs[0].components || []).map((x, i) => (x.name || "").trim() || `unnamedOutput${i}`)
: []);
} else runtimeKeys = outs.map((o, i) => (o.name || "").trim() || `unnamedOutput${i}`);
for (const d of a.outputs || []) {
const nm = (d.name || "").trim();
if (nm && !runtimeKeys.includes(nm)) { phantom++; byProto[p.slug] = (byProto[p.slug] || 0) + 1; }
}
}
console.log(phantom, Object.keys(byProto).length, JSON.stringify(byProto));
'
# 204 19 {"sky":31,"yearn":27,"chronicle":26,...}
```
This is not only autocomplete. `buildOutputFieldsFromAction` feeds `outputFields`, which `lib/mcp/output-schema.ts:27` turns into the published MCP `outputSchema` via `lib/action-schemas/builder.ts:169`. An agent calling `list_action_schemas` is told `balance` is a property of the output, writes a template against it, then gets undefined with no error.
**The workaround** is for a user to ignore the suggestion, run the action once, read the raw result and hand-write `{{Node.result}}`. That is only discoverable after the workflow silently does nothing.
### Scope: what this touches, and what it does not
**In scope:** `buildOutputFieldsFromAction` in `lib/protocol-registry.ts`, plus a registry-wide test asserting every advertised field is reachable in the runtime shape.
**Not in scope, deliberately:**
- **No runtime change.** `structureAbiOutputs` is shared by `read-contract-core.ts:333`, `batch-read-contract.ts:281` and `batch-write-contract-core.ts:269`. Renaming outputs there would change `result` from a bare scalar to an object for raw web3 nodes too, breaking every existing `{{Node.result}}`. The advertisement is what is wrong, so the advertisement is what changes. This matches the direction KEEP-296 took for writes.
- **No protocol definition edits.** The 204 figure is a consequence of one function, not 19 separate authoring mistakes. Nothing in `protocols/` needs to change for this fix.
- **`decimals` and `label` are kept.** They are real metadata and still describe the value; only the advertised path changes.
- Write actions, already correct per KEEP-296.
**Surfaces checked and found fine:** raw web3 single/multi/tuple outputs via `action-output-fields.ts` (correct already); the calldata goldens (inputs only, they would not catch this); the tier-1 sweep (only 4 protocols declare a `field:` expectation on an affected action, which is why this survived).
### Plan
1. Derive the advertised path from the same source the runtime uses, the ABI output shape, rather than from the override name. Concretely: prefix `result.`, then emit a named subfield only where `structureAbiOutputs` will actually produce that key. Where the ABI output is unnamed and scalar, the only correct suggestion is `result`. `action-output-fields.ts:82-108` is the reference implementation, so I would match its behaviour rather than invent a second one.
2. A registry-wide invariant test walking every registered read action and asserting each advertised `outputField` is reachable in the shape `structureAbiOutputs` produces for that ABI. It fails on 204 fields today and is the regression gate. No such test exists, which is the reason this reached 19 protocols.
Happy to adjust the shape before writing anything, particularly on one point: whether a scalar read should advertise bare `result`, versus having the override name become real by naming the ABI output in the protocol definitions. The first is a one-function change and breaks nothing. The second makes the nicer names work but changes the runtime result shape for those actions, which is a migration.
### Plan: alternatives considered
- **Change the runtime to honour the override name.** Rejected as the default: it changes `result` for existing workflows on 204 actions. `structureAbiOutputs` is shared with raw web3 nodes, so the blast radius is larger than the bug.
- **Drop `action.outputs` from the advertisement entirely**, mirroring writes exactly. Rejected: it is strictly worse for users, since the tuple and named-output cases are correct today and genuinely useful.
- **Fix the 19 protocol files by naming every ABI output.** Rejected: 204 edits to fix one function, plus it changes the runtime shape of shipped actions.
### AI assistance
AI assistance (Claude, Anthropic) was used to write the audit script and draft this issue. The reproduction was run against `staging` at `f8c8f18c7` and the counts, the file references and the Lido trace were verified by the author before filing.
Contributor guide
Research direction
Start with buildOutputFieldsFromAction in lib/protocol-registry.ts, then compare its behavior with lib/workflow/editor/action-output-fields.ts and structureAbiOutputs in plugins/web3/steps/structure-abi-result.ts. Add the registry-wide invariant test described in the issue, using the registered read actions and their ABI shapes. Done means every advertised output field is reachable at runtime without changing result values or write actions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, backend, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100