Skip to content

Two follow-ups from #1095: a comment cites the wrong file/line, and MENTION_RAW_TYPES contradicts its own superset rule #1108

Open

Description

@lilyshen0722

Both were non-blocking findings in my review of #1095 and shipped with it at 730608b4. Filing so they don't evaporate with the merged PR.

1. The in-file citation resolves to unrelated code

backend/services/nativeRuntimeService.ts:512-513:

agentEventService:994 forwards type verbatim, and the claimable-set gate at :697 in this same file says so out loud ("raw-type gate mirrors the wrapper's claimable set")

The gate carrying that comment is at nativeRuntimeService.ts:740 — the same file as the citing comment, but line 740, not 697. agentEventService.ts:697 is the #993 pending-delete discussion, unrelated. And because the preceding clause names agentEventService, "this same file" reads as that file, so a reader resolving the pointer lands on the wrong code whichever they open.

The claim it supports is true — I verified agentEventService.ts:944 gates on runtimeType === 'native' and :994 forwards type verbatim. Only the pointer is wrong.

Fix: nativeRuntimeService.ts:740, and drop "in this same file" or make it unambiguous.

2. MENTION_RAW_TYPES omits message.posted, against its own stated rule

The set at :488 documents itself as err-wide:

Deliberately a superset of what matches today: a name in here that never arrives costs nothing, a name missing from it costs a discarded cue, and this file has already paid that once.

But:

MENTION_RAW_TYPES    chat.mention, thread.mention, dm.message, mention, chat.message
CLAIMABLE_RAW_TYPES  chat.mention, thread.mention, dm.message, mention, chat.message, message.posted

Two raw-type vocabularies in one file differing by one member — structurally the bug #1095 fixed, one scope smaller.

Impact is small: board wakes always carry payload.content, so only a content-less message.posted is affected, and it lands on the generic "Trigger: X, use commonly_read_context" fallback — the output #1095 identifies as the defect. But by the set's own rationale the member costs nothing to add.

Fix: add 'message.posted', or amend the comment to say why this set is deliberately narrower than its sibling.

Test gap worth closing with #2

Nothing asserts that message.posted with content and without content take different paths. The content-ful case is covered by the parametrised suite; the content-less one is exactly this blind spot.


Both are one-liners. Raising #1 with slightly more weight than its size warrants because it is the fourth load-bearing miscitation found this week — TASK-025's title, TASK-026's spec and ADR-024 all cited poller.js for a loop no seat executes — and a wrong pointer inside the fix for a discovery bug is the last place it should survive.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions