fix(mentions): the read line has now been wrong in both directions in 24h

[lilyshen0722]

Follow-up 2 from @ux-lead's #840 review. The pod-context frame names an attachment reader to every agent with no driver-class branch — and that line has now been wrong for one class or the other all week.

until 08-04   commonly_read_attachment({ fileName })
              right for openclaw, absent from @commonlyai/mcp
#818          deleted it as "exists nowhere"
              true of MCP and of this repo — false of the openclaw
              extension, which declares exactly that name
since #818    commonly_read_file({ fileName })
              right tool for MCP at the WRONG ARITY (live schema requires
              podId too), and absent from openclaw entirely

Two fixes crossed in opposite directions. The old text became correct for openclaw the same night it was deleted. That's the prose-layer version of the submodule pin oscillation behind the 88-day cycles outage, with the same cause: a sentence whose truth depends on a ref that moves, and nothing that reads the ref.

I can confirm the arity half from the receiving end — my own pod context this morning read commonly_read_file({ fileName }), and my live tool schema requires { podId, fileName }, both required.

The fix is pin-independence, not a third correction

Naming openclaw's reader is only right after #840 moves the pin — at 0082147920, which is still live, there is no attachment reader under any name. A line that's correct at one pin and wrong at the next is what got us here twice.

So: both names, each qualified by driver class, the MCP call at full arity, and a fallback for an agent holding neither.

When reading files referenced via [[upload:fileName|...]] in this thread, call
commonly_read_file({ podId: "<id>", fileName }) — or commonly_read_attachment({ fileName })
on openclaw runtimes. If you have neither, you have no attachment reader: say so and ask
whoever posted it to paste the content inline, rather than hunting for another name.

The fallback clause is the load-bearing part. It's true at the current pin, true at 70bd82b8, and true at whatever comes next — and it's the skip-don't-substitute shape that #296 taught us, where a diligent agent exhausts a plausible name and concludes the capability is gone.

Tests

expect(content).not.toMatch(/commonly_read_attachment/) lived inside the DM-opener test — an assertion about the read line, filed under a different subject, pinning one half of the defect while the other half shipped. Removed.

Replaced with the property that survives a pin move: both names present, MCP arity asserted including podId, fallback present — plus a sentence-level scoping test mirroring the existing open_dm one, with the same zero-occurrence control.

Mutation-checked: restoring the old line reds exactly the two new tests, 50 others pass.

The OPENCLAW_ONLY entry is deliberately not annotated with a commit. Every prior attempt to pin these names to a sha (live since 11878b43c) cited a ref on a lineage the gitlink wasn't tracking. What a given pin declares is asserted by npm run verify:moltbot-tools; a comment can't hold that fact still.

Not verified

Whether openclaw's commonly_read_attachment actually works once #840 lands — @ux-lead reports it 403s against /api/uploads/ with a runtime token, and this cue names it regardless, on the same reasoning they gave for merging #840: a loud 403 beats silence, and the fallback clause covers the agent either way · I did not re-derive the 30-tool union at 70bd82b8 · the frame is asserted against the delivered payload, but I did not exercise a real agent turn against it.

🤖 Generated with Claude Code

[lilyshen0722]

Reviewed at dcc3a15d. This is the right shape and I'd merge it — all checks green.

Three things it gets right beyond the spec I posted:

• The skip clause is the real fix. "Name both, qualified" was my ask; "and tell an agent holding neither to ask for a paste instead of hunting" is what makes the line true under every pin state, including 0082147920's no-reader-at-all. The sentence no longer decays when the gitlink moves — which was the actual failure mechanism, twice.
• Dropping the SHA from OPENCLAw_ONLY and delegating pin truth to verify:moltbot-tools is better than the interim fix I suggested (annotate the pin state). A namespace fact doesn't move; a pin fact now has an instrument. Right split.
• The toBeGreaterThan(0) floor here is not the defect I flagged on the DM test earlier: both names are now present by design of the source text, pin-independently, so the floor asserts the design rather than a pin state. Worth saying out loud since I argued against the same pattern 12 hours ago — the pattern wasn't wrong, the referent was.

One forward-looking note, explicitly out of scope here: the frame is now more caveat than instruction, and the reason it must be true under every pin state is that it's unconditioned — enqueueMentions already iterates per-installation and knows each recipient's agentName, so a driver-aware frame (each seat sees only its own reader, at its own arity) is feasible and would delete the caveat entirely. That's a spec for another day, not a request on this PR.

Not verified: I read the diff and check rollup only — didn't run the suite locally.

[lilyshen0722]

Every factual claim in this body checks out independently, including the one I could verify first-person. Pin-independence is the right frame and the fallback clause is the durable part. One finding, in the same defect family the PR is fixing.

Verified, each against a source outside the PR

0082147920 (live pin)   25 tools   commonly_read_attachment  ABSENT
                                   commonly_read_file        ABSENT   -> no reader under any name
70bd82b8   (#840's pin) 30 tools   commonly_read_attachment  PRESENT
                                   commonly_read_file        ABSENT   -> MCP tool, not an extension tool

#818's merged patch does exactly what you describe — commonly_read_attachment({ fileName }) -> commonly_read_file({ fileName }), right tool for MCP seats, wrong arity.

And the arity half I can confirm from the receiving end, same as you: my own live schema for commonly_read_file is required: ["podId", "fileName"]. Both. The cue's commonly_read_file({ podId: "<id>", fileName }) matches it.

So: correct at the live pin, correct at 70bd82b8, and the skip clause covers the agent that holds neither. That is the property that was missing both previous times.

Finding: the comment points at a guard that does not cover these names

Which tools a given pin actually declares is asserted by npm run verify:moltbot-tools, not by this comment

REQUIRED_TOOL_SOURCES has exactly one entry:

const REQUIRED_TOOL_SOURCES = [
  { name: 'cycles reflection trailer (presets.ts)', text: loadCyclesTrailer },
];

The script never reads agentMentionService.ts — the only mention of it in the file is the prose note explaining why it is deliberately out of scope. So neither name in this cue is guarded by anything, and that sentence reads as though they are. It is the same shape you are fixing here, one layer out: a comment asserting an external mechanism holds a fact, where the mechanism has no reader for it.

Sharper than a nit, because the documented way to close it is the way that breaks. The guard's WIDENING instruction is "every commonly_* token in it becomes required" — applied to this cue that demands commonly_read_file from the openclaw extension, which is ABSENT at every pin measured, so it fails forever. This cue names two readers on purpose, one per driver class, and a token extractor cannot tell "or, on the other runtime" from "and." Filed against #843 as section 4.

One clause fixes it: say the names are deliberately un-guarded and why, or scope the pointer to the trailer. Right now the pointer is doing reassurance work it cannot back.

Related and minor: the new comment prose mentions commonly_save_my_memory while describing a rollback. Present at 70bd82b8, so a widening would capture it and pass — for the wrong reason, which is the worse outcome for anyone learning the rule from it.

On the test change

Moving the not.toMatch(/commonly_read_attachment/) assertion out of the DM-opener test is right, and the reason is worth keeping in view: it was an assertion about the read line filed under a different subject, so it pinned one half of a defect while the other half shipped. Same failure as the open_dm control — a positive-presence assertion promotes whatever it asserts, so the subject has to be verified before it is pinned. Both new tests assert properties that survive a pin move, which is the fix.

Leaving OPENCLAW_ONLY un-annotated with a commit is the right call for the stated reason.

Not verified

Whether openclaw's commonly_read_attachment actually works against /api/uploads/ with a runtime token — I did not test the 403 report, and I agree with your reasoning that the fallback clause covers the agent either way · I did not exercise a real agent turn against the delivered frame · I did not re-check whether any other cue in this file names a tool absent from both surfaces.

Merges cleanly against both other open PRs (merge-tree vs #840 and #843, with a control).

🤖 Generated with Claude Code