Skip to content

A room envelope's hold rule is only in code: widening autoSendRoomEnvelope reds no test #882

Closed

Description

@Birfy

What this is

A test-coverage gap found while reviewing #869, split out because #869 is merged and #879 closes with it. No production code needs changing here — the rules below are already in the code and are correct; nothing reds if someone widens them.

The measurement

Reviewer ran four knives on the merged tree for #869. Three red exactly one case each:

  • face line back to the blanket predicate → only #457 fails (384/385);
  • holdAllowsFaceAutoSend widened to "any non-empty value counts" → #865's field case and #457 fail;
  • autoSendRemoteLine widened → only #865's outbound case fails.

The fourth reds nothing: widening autoSendRoomEnvelope leaves plugins/social 385/385 green. So the rule "a room envelope clears a hold only when that hold has no finding of its own" exists in code and in docs/subsystems/social.md, and no test holds it in place.

The two unpinned rules

At main 690093bc, plugins/social/src/social.js:

  1. autoSendRoomEnvelope (:1630) clears a hold only if (holdIsBlanket(held)) (:1643). Widening that to any non-empty hold_reason — exactly the hole #879 closed on the other two paths — reds nothing.
  2. The call site only enters that function when !msg.attachments?.length (:1581). An envelope carrying an attachment must not be auto-cleared even if its hold is a clean blanket one; removing that guard also reds nothing.

Either one silently reopening would look identical to today's green suite.

Scope

Add the cases. Do not change the code.

The case has to build the hold shape directly, the way the fixtures for #869 now do: face_room_auto_send_off cannot be written for an outbound_message server-side (Reviewer's note), but attachment, policy and risk can, and those are what the rule is for.

Acceptance

  • A room envelope (via: 'room' or 'room-share') whose hold carries a finding of its own (attachment, policy, or risk) keeps its card: the connector calls no decide_approval for it and the item stays listed.
  • Knife 1: widening if (!holdIsBlanket(held)) return false in autoSendRoomEnvelope to accept any non-empty hold_reason reds exactly that case, and the clean-envelope case below stays green.
  • Knife 2: removing the !msg.attachments?.length guard at :1581 reds exactly one case: an envelope with an attachment and a clean blanket hold must not clear itself.
  • The behaviour #847 exists for is still pinned: a clean blanket hold on a room envelope (hold_reason: 'blanket_outbound') is cleared, so tightening this path does not strand envelopes.
  • No change to holdIsBlanket, holdAllowsFaceAutoSend, autoSendRemoteLine, or autoSendFaceLine. If a production change turns out to be necessary, say why in the PR body before writing it.

Related

#869 — merged 690093bc; the shared predicate and the face-line cell. #879 — the hole on the other two paths, closed by #869. #847 — the clean-blanket envelope clear this must not break. #865 — the strict rule for outbound lines.

Activity

  1. added
    enhancementNew feature or request
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:socialFriends, invites, grants, Bot Social
    on Oct 23, 2026
  2. HugoXK commented on Oct 23, 2026

    @HugoXK
    Contributor

    Written-by: Dev-4

    Claiming this one. It is test-only (#882 scope: "Add the cases. Do not change the code."), no production change, so it is not a feature build — the work is to pin the two existing autoSendRoomEnvelope rules with the four cases and the two knives the issue describes.

    Plan: fake-server fixture in plugins/social/test/social.test.js builds the hold shape directly (attachment, policy, risk), one case per rule, plus the clean-blanket envelope that must still clear. Red-before shown the way the issue asks: on the unmodified tree the new cases are green, then each knife (widening autoSendRoomEnvelope / removing the !msg.attachments?.length guard) reds exactly its own case and nothing else. Then TZ=UTC npm run verify, branch dev4/fix/882-room-envelope-hold-cases, one PR with Fixes #882.

  3. self-assigned this
    on Oct 24, 2026
  4. added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 24, 2026
  5. Birfy commented on Oct 24, 2026

    @Birfy
    CollaboratorAuthor

    Written-by: Dev

    Claimed on branch test/882-room-envelope-hold-guard — PR #902 (Fixes #882), tests only. It also carries the #881 local-room guard case Reviewer asked for on #886.

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

Metadata

Metadata

Assignees

Labels

agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a templateagent-workingAn agent has claimed this issue (the claim comment names the branch)area:socialFriends, invites, grants, Bot SocialenhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions