Skip to content

fix(social): a room approval card waits for its own list before naming another owner - #1623

Merged
Birfy merged 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev/1607-room-approval-card-can-show-waiting-for
Oct 29, 2026
Merged

Birfy merged 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev/1607-room-approval-card-can-show-waiting-for

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1607
Owner: Dev2
Reviewer: Reviewer
Written-by: Dev-2

Base: 63f07ef (live main, read just before the branch was cut)
Tip: 37202a6
Tree: 929512775ecc40e9f651b0bc9e1c1f01d2163b92 (read back from GET /commits/37202a61…)

What was wrong

ChatApprovalDock decides "this card is somebody else's" from mine, which is built from two lists that poll on their own 5 s timers: social/status (items) and social/approvals (local). waitingOthers was mine.length === 0 && room.pendingCount > 0, so an empty mine was read as "no card of ours" while it equally meant "neither list has answered yet". room.pendingCount only says the room holds a pending card; it never said whose.

The window is the one in the report: the list lags by a poll, the dock draws the no-button Waiting for the owner's approval card for a card that is in fact ours, and one poll later the buttons appear. The sentence is false when it is drawn — nothing is waiting for another owner; that owner's own machine is holding the decision.

What changed

plugins/social/client.js, one component (ChatApprovalDock), no server change:

  • Each list now records that it has answered (statusLoaded, listLoaded), set in the same then that stores its rows. A failed call leaves its flag false.
  • waitingOthers needs both flags, no entry for this room in either list (room.id or room.face.roomId), and room.pendingCount > 0. Until then the dock draws nothing, which is the neutral state the issue asks for.
  • The two named(...) lists are built once (held) and mine filters that, so the room check and the card see the same rows.

Why this shape and not deleting the branch: the honest case is real — a face room where another home holds the card — and dropping the branch would leave the owner with no hint at all that the room has a pending card. Gating it on the lists having answered keeps that case and removes the guess. An entry for this room that this chat does not draw (a held reply decided on its own bubble, #418, or in the Bot's chat) also suppresses the card: that card is ours, not another owner's.

docs/subsystems/social.md states the new condition in the dock paragraph; the changelog fragment is changelog.d/1607-approval-dock-waiting.fixed.md.

Red → green, honestly

Two commits, the test one not folded into the fix:

  • 583dd3bbdf7c — test only: plugins/social/test/approval-dock.test.js. On the base 63f07ef6 (client at main) it is 3 tests, 2 pass, 1 fail: #1607: a dock whose lists have not answered yet claims no other owner and draws no buttonless card — the dock renders [no-buttons]Waiting for the owner's approval. The other two are the locks: our own card becomes Approve / Reject once the list answers, and the other-owner card still appears once the list is answered and empty.
  • 37202a6134a9 — the fix, on top of it: client.js, social.test.js (the #114 dock test now states the two flags explicitly instead of relying on a default hook slot), the subsystem doc and the fragment.

At the tip: test/approval-dock.test.js 3/3, plugins/social 531/531.

Files

5 files changed, 152 insertions(+), 6 deletions(-)
 plugins/social/client.js                         |  20 ++++--
 plugins/social/test/approval-dock.test.js        | 131 +++++++++++++++++++++++
 plugins/social/test/social.test.js               |   4 +-
 docs/subsystems/social.md                        |   2 +-
 changelog.d/1607-approval-dock-waiting.fixed.md  |   1 +

Checks

TZ=UTC ARCH_DOCS_BASE=63f07ef6 npm run verify at tree 92951277… — exit 0: core 1903, social 531, tree 125, timeline 129, workflow 226, .github 549, interop 37, live-e2e 24; oxlint 0 errors (only the repository's pre-existing warnings); build-client --check clean; arch-docs: OK (5 changed file(s)); ds-bot-patches check: ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/; node .github/scripts/changelog.mjs check — fragments well formed.

Not run: no live instance, no browser (the dock is covered by the client bundle test); CI was not waited on.

Checkpoint

{
  "pr": 1623,
  "claim": "Dev2",
  "issue": 1607,
  "summary": "The room approval dock read an empty `mine` as 'no card of ours' while it also meant 'neither list has answered yet', so it drew the no-button 'Waiting for the owner's approval' card for a card that was ours. Both lists now record that they answered, and that card needs both flags, no entry for this room in either list, and a pending count. Until then the dock draws nothing. The honest other-owner case and our own Approve / Reject card are pinned by the same test file.",
  "files": [
    "plugins/social/client.js",
    "plugins/social/test/approval-dock.test.js",
    "docs/subsystems/social.md"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=63f07ef6 npm run verify, exit 0 (core 1903, social 531, tree 125, timeline 129, workflow 226, .github 549, interop 37, live-e2e 24, oxlint 0 errors); approval-dock.test.js red first on 63f07ef6 2/3, green 3/3 at 37202a61",
    "result": "green",
    "at": "37202a6134a9bafbb9fd28e1ba3d7fdf54055afe"
  },
  "next": "none — ready for review"
}

@HugoXK
HugoXK marked this pull request as ready for review October 29, 2026 02:42
@HugoXK

HugoXK commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy — formal review request for #1623 at tip 37202a6134a9bafbb9fd28e1ba3d7fdf54055afe.

Ready for review (not a draft). Red first on the pinned base 63f07ef6d2330fe42fa6670f15fa81436d7a02c5: plugins/social/test/approval-dock.test.js at commit 583dd3bbdf7c (test-only, first commit of this branch) is 3 tests / 2 pass / 1 fail — #1607: a dock whose lists have not answered yet claims no other owner and draws no buttonless card renders [no-buttons]Waiting for the owner's approval. The fix commit 37202a6134a9 sits directly on top; at this tip the file is 3/3 and plugins/social is 531/531.

Checks at this tip: tests-ok, plugins (social|tree|timeline|workflow), package, judge, merge-gate, ubuntu-latest all success. enqueue-guard is red with just no Verifier report names the tip 37202a6134a9 — that is the verdict this request is asking for.

One implementation note for the reviewer: the buttonless branch was not deleted — waitingOthers now requires both lists to have answered and neither to hold an entry for this room, so a genuinely other-owner pending card still shows. maintainer_can_modify: true.

HugoXK added 2 commits October 29, 2026 03:05
…e list (ThinkFlowLab#1607)

The room approval dock reads social/status and social/approvals on their own timers, so an empty `mine` can mean 'not loaded yet'. This test holds the approval list back and asserts the dock draws no buttonless 'Waiting for the owner's approval' card before it answers; it is red on the current client and green once the wait is gated on the lists having answered.
…hinkFlowLab#1607)

ChatApprovalDock built `mine` from two lists that poll on their own timers, so an empty `mine` could mean 'not loaded yet' or 'somebody else's card'. The no-button 'Waiting for the owner's approval' card now needs both lists answered and neither holding a card for this room; until then the dock draws nothing. The ThinkFlowLab#114 dock test states the answer explicitly instead of relying on a default hook slot.
@Birfy

Birfy commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 37202a6
Written-by: Reviewer
Attestation: 37202a6 role=Reviewer

Ran at this tip:

$ cd /data/dsh/bot/workspace/p1623tip/plugins/social && TZ=UTC node --import ./test/resolve.mjs --test test/approval-dock.test.js
# tests 3  # pass 3  # fail 0
$ cd /data/dsh/bot/workspace/p1623tip/plugins/social && TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 531  # pass 531  # fail 0
$ cp /data/dsh/bot/workspace/p1623base/plugins/social/client.js /data/dsh/bot/workspace/p1623red/plugins/social/client.js
$ cd /data/dsh/bot/workspace/p1623red/plugins/social && TZ=UTC node --import ./test/resolve.mjs --test test/approval-dock.test.js
# tests 3  # pass 2  # fail 1

The last two commands are the red-first check: the same tip tree with only plugins/social/client.js swapped back to the base 63f07ef6 blob (git is not on this box's PATH, so the base tarball's file was copied in). Exactly the one new #1607 test fails — approval-dock.test.js "a dock whose lists have not answered yet claims no other owner and draws no buttonless card" — and the two lock tests stay green. The green runs above are on the untouched tip tree.

Scope: tip vs base 63f07ef6 is ahead 2 / behind 0, 5 files +152/−6 — changelog.d/1607-approval-dock-waiting.fixed.md, docs/subsystems/social.md, plugins/social/client.js, plugins/social/test/approval-dock.test.js, plugins/social/test/social.test.js. No core/, no generated file, no server change.

Review of the change:

  • ChatApprovalDock now records that each of its two polls answered (statusLoaded set in the social/status .then, listLoaded in the approvals .then), so a rejected call leaves its flag false and the dock keeps waiting rather than guessing.
  • waitingOthers requires both flags, no held entry for this room (room.id or room.face.roomId), and room.pendingCount > 0; until then the dock renders nothing, which is the neutral state the issue asked for. The honest other-owner case still draws once both lists answer empty.
  • held is built once from both lists and mine filters it, so the room check and the card read the same rows; the #418 own-bubble case is preserved — an entry for this room that this chat does not draw still suppresses the waiting card.
  • docs/subsystems/social.md states the condition in its dock paragraph and the fragment is one .fixed.md line; the #114 dock test in social.test.js now states the two flags instead of relying on a default hook slot.

Checks at this tip: 16 check runs, all success except the one enqueue-guard run at 02:41:00Z, which is the pre-PASS refusal (no Verifier report names the tip 37202a6134a9); this comment is that report. This is a fork PR, so the Owner:/Reviewer: and agent-pr rules are exempt, but T2/T3 still need this PASS: it is the gate.

@github-actions github-actions Bot added the pass label Oct 29, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 29, 2026
Merged via the queue into ThinkFlowLab:main with commit 8f6e60c Oct 29, 2026
15 of 16 checks passed
@Birfy
Birfy deleted the dev/1607-room-approval-card-can-show-waiting-for branch October 29, 2026 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Room approval card can show "Waiting for the owner's approval" with no buttons while the approval is already pending

2 participants