Repository navigation
fix(social): stop announcing a room's own pending card as another owner's - #1639
Conversation
cd51024 to
1b41995
Compare
…er draws ChatApprovalDock decides "somebody else is waiting" from two lists it polls every 5 s, while the room object it is handed already carries `pendingApprovals` — the same array `pendingCount` counts in core/src/host/channel.js. One poll behind, a room's own card was drawn as the no-button "Waiting for the owner's approval" card. Gate that line on the room's own list holding no card this dock would draw, checked with the dock's own rule (`approvalsForOpenChat`; inline holds are left out, as `held` already drops them). Reading only "the room list is not empty" would silence the line for every room card, including the ones this chat never draws: a held line decided on its own bubble (ThinkFlowLab#418) or in the Bot's chat (ThinkFlowLab#254) keeps it, because there the sentence is true. The dead `thisRoomHeld` guard goes: its only input was `held`, whose room rows `approvalsForOpenChat` already turns into `mine`, so it could never fire. Fixes ThinkFlowLab#1638 Owner: Dev3 Reviewer: Reviewer Written-by: Dev-3
1b41995 to
5e5356a
Compare
|
@Birfy — review request for PR #1639 at the current tip Scope:
As of my read: 0 reviews, 0 comments on this PR; head is |
|
Verdict: PASS at 5e5356a Ran at this tip: Red first: with only the changed source
|
…jump (ThinkFlowLab#1641) Fixes ThinkFlowLab#1640 Written-by: Dev-1 Owner: Dev Tip: 778cd71 Tree: 5160592 ## What happens The dock draws "Waiting for the owner's approval" exactly when the room has a pending card that this chat does not draw: a held reply decided on its own bubble (ThinkFlowLab#418), or a Bot's hold decided in that Bot's chat (ThinkFlowLab#254). The card carried only an id, so it showed its label and nothing else — no name, no way there — on screen precisely when the person cannot act where they are. ## What the card says now `plugins/social/client.js` (`ChatApprovalDock`), inside the `waitingOthers` branch only; the guard, the two polled lists, `mine` and real cards are untouched (that is `ThinkFlowLab#1638`/`ThinkFlowLab#1607`'s work): - a held reply (`request.via === 'auto-reply'` with `request.meta.replyIn.sessionId`, the same derivation `localHoldView` already does) → **Decide on the reply bubble above.**, no button: the bubble is already on screen; - a hold written by a local Bot (`fromId`) or addressed to a listed friend's chat → **Decide in {Bot}'s chat.** plus one button, **Open {Bot}'s chat**, which calls the dock's existing `actions.openSession(<that chat id>)`; - anything the dock cannot place → today's neutral sentence, and no button. A destination is never invented. Both languages: the two new sentences go through the host translator with their zh entries added to core's dictionary (`'Decide on the reply bubble above.'` → `在上面的回复气泡里批准。`, `"Decide in {name}'s chat."` → `在 {name} 的聊天里批准。`); `"Open {name}'s chat"` was already in the dictionary and is reused. The card's own label stays the English constant it was. ## Red → green The first commit of this PR is the tests only, `540c14df38c627e0e9e2117a12e0ef524064b987` (tree `7bd2cd4e…`), and stays in history as its own commit. On it `plugins/social/test/approval-dock.test.js` is 10 tests / 8 pass / 2 fail: `ThinkFlowLab#1607`'s three cases and `ThinkFlowLab#1638`'s four pass, and the two failures are exactly the new `ThinkFlowLab#1640` assertions, both reading `[no-buttons]Waiting for the owner's approval`: - `a hold written by a local Bot names that Bot's chat and opens it` - `a held reply names the reply bubble above and offers no jump, because it is on screen` On this tip the file is 11 tests / 11 pass / 0 fail — those seven untouched, the three new cases above, and the fourth case the last commit adds (a held reply from another chat). The third new case (a room count the dock cannot place keeps the neutral sentence and no button) passes on both tips on purpose: it is the "never invent a destination" guard. The green commit also fixes the new test's tree walker so it mounts a component to reach the button inside the card; the red assertions and the card's behaviour are unchanged by that. The red push happened while live main advanced `3280258e → 1ed614a`; the branch was overlaid on the new base without overlap, then rebased locally, and the pushed red tree (`34d2b92ae4`) is byte-identical to the local one. Later the same command still passed with the base moving to `c26089f5` (which merged `ThinkFlowLab#1639`, a change to this very file), so the branch was rebased: every commit was rewritten and the branch force-updated, which voids the PASS written at `b49ffa2e`. The conflict was resolved by keeping both sides — `ThinkFlowLab#1639`'s room-card rule (`roomCards`, read from the room's own `pendingApprovals`, stays the guard) and this PR's `waitingPlace()`, scoped to the same room — and the generated bundle and patches were rebuilt from the merged sources (`npm --prefix core run build`, then `node scripts/ds-bot-patches.mjs generate`), never hand-merged; the generator rewrote them from the merged sources, and `build-client --check` and `ds-bot-patches check` are clean at the tip. The red commit was re-run on the new base and fails the same two assertions. A third commit tightens one detail: `local` holds every local approval, so a held reply drawn in another chat must not be reported as this room's bubble. It matches the reply's own session now and its test is red on the previous tip (`a held reply from another chat is not this room's bubble`) and green here; the diff is two lines plus that case. ## Generated artifacts `core/client.js` is the committed bundle rebuilt from `core/src/client` with `npm --prefix core run build` (the dictionary lives in the bundle), and `patches/ds-bot/0001-core-fixes.patch` + `0008-client-bundle.patch` come from `node scripts/ds-bot-patches.mjs generate`; the other patches the generator rewrites came out byte-identical to the base. ## Checks run `TZ=UTC ARCH_DOCS_BASE=c26089f5 npm run verify` → **exit 0** on this tip: - core 1924 / social 539 / tree 125 / timeline 129 / workflow 234 / `.github` 549 / interop 37 / live-e2e 24 tests, **fail 0** in every suite; - `build-client --check` clean; - `arch-docs: OK (7 changed file(s))` — baseline ref `ARCH_DOCS_BASE=c26089f5`, the branch base; - `ds-bot-patches check`: `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ + patches/ds-bot/`; - `lint-plugins`: 0 errors (warnings only, unchanged from main). `Docs-not-needed: the change names a destination inside the existing waiting card and adds two dictionary strings; docs/subsystems/core-client.md and social.md describe structure, not this card's copy.` ## Checkpoint <!-- flybot:checkpoint --> { "pr": 1641, "claim": "Dev", "issue": 1640, "summary": "The dock's waiting-for-approval card named no destination even though it is drawn exactly when the room's card is decided elsewhere, so the person saw a label and no way to act. It now names a held reply's bubble above (no button, it is on screen) or a Bot's chat (one button that opens it through the existing openSession), and a card it cannot place keeps the neutral sentence with no button. Both new sentences carry zh copy in core's dictionary and reuse the existing Open {name}'s chat label.", "files": ["plugins/social/client.js", "plugins/social/test/approval-dock.test.js", "core/src/client/i18n.js", "core/client.js", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/0008-client-bundle.patch", "changelog.d/1640-waiting-card-destination.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=c26089f5 npm run verify", "result": "green", "at": "778cd7132543ec29ec22aa3c1b16bad9cf051758" }, "next": "none — ready for review" } <!-- /flybot:checkpoint -->
Fixes #1638
Owner: Dev3
Written-by: Dev-3
Reviewer: Reviewer
Branch:
dev/1638-social-a-room-s-own-pending-card-is-annoBase:
1ed614a7fd4ac4ba842be00b425c7e3f49525d40(main)Tip:
5e5356a04824243a90e0884ac6b39612adcc804dTree:
4e00f840ab1791f83ce8fc9d71a142024f5be808What was wrong
A room holding one of our own pending cards could draw the no-button Waiting for the owner's approval card instead of our Approve / Reject card, for one poll.
ChatApprovalDockbuildsminefrom two lists it polls itself every 5 s (social/statusandapprovals). While those are one beat behind, a card that is ours is not inmine, and the only other input wasroom.pendingCount > 0, so the dock narrated our own card as another owner's. The sentence is false when it is drawn: nothing is waiting for another owner; this owner has to decide.The room object the dock is handed already carries the answer. In
core/src/host/channel.jspendingApprovals: pendingandpendingCount: pending.lengthare the same array, so the room view knows its own card one poll before the dock's two lists do.The line is narrowed, not switched off
room.pendingCount > 0androom.pendingApprovals.length > 0are the same array, so gating the line on "the room list is not empty" alone would makewaitingOthersunreachable and would also silence the rooms where the line is true. The two classes are told apart by the dock's own drawing rule, not by a guessed field:mineis empty — only these two 5 s polls are behind. Stay quiet for that beat; the next poll draws the real Approve / Reject card (social: a room's own pending card is announced as "waiting for the owner's approval" #1638).What changed
plugins/social/client.js, one component (ChatApprovalDock):const roomCards = approvalsForOpenChat((room?.pendingApprovals ?? []).filter(item => !item.inline), session)— the room's own list through the same helper that buildsmine, so the rule is literally the dock's.waitingOthersnow needsroomCards.length === 0as well.thisRoomHeldguard is gone. Its only input washeld, whose room rows (room.id/room.face.roomId)approvalsForOpenChatalready turns intomine, so it could never fire whenmine.length === 0; keeping it would be a comment promising a check nothing performs.docs/subsystems/social.mdstates the narrowed condition in the dock paragraph;changelog.d/1638-approval-dock-room-card.fixed.mdrecords it.On the
inlinefilter: the room list is Bot Social'spendingCache, andinlineis set only by the host'slocalHoldViewfor a card decided on its own bubble (#418) — the room list carries no such marker today. The filter is there so the room check stays exactly the dock's own rule (heldalready drops those,plugins/social/client.js~:2422); the bubble test below pins that the two agree if one ever arrives. Nothing else is inferred from the item shape.Tests
plugins/social/test/approval-dock.test.jsgains four cases; the three#1607cases are unchanged and stay green:pendingApprovalsholds its ownface_room_messagewhile both dock lists answer empty for one poll — noWaiting for the owner's approval, no buttonless card.inline) in the room list — the waiting line stays; the dock decides nothing it does not draw.outbound_message, decided in the Bot's chat) — the waiting line stays.pendingCount: 0, nopendingApprovals— draws nothing.The last three are guards: they pass before and after, and the two "line stays" cases are what the first cut of this fix got wrong (see Evidence), so they are load-bearing for class 2.
Evidence
Red first, on the test-only tree (source untouched, live
main1ed614a7plugins/social/client.js), test-only commitcfa646e2on the local branch and squashed into the tip by the push helper —cd plugins/social && node --import ./test/resolve.mjs --test test/approval-dock.test.js, 7 tests / 6 pass / 1 fail:The narrowing is measured too: with the earlier check
roomHoldsCard = room.pendingApprovals.length > 0, the same file is 7 tests / 5 pass / 2 fail —#1638: a room card drawn on its own bubble keeps the waiting lineand#1638: a room card this chat never draws keeps the waiting line. That is why this tip replaces1b41995crather than keeping the first cut.Green at
5e5356a04824243a90e0884ac6b39612adcc804d:$ TZ=UTC ARCH_DOCS_BASE=1ed614a7fd4ac4ba842be00b425c7e3f49525d40 npm run verify— exit 0: core 1918, social 535 (was 531, +4), tree 125, timeline 129, workflow 234,.github549, interop 37, live-e2e 24, every suite# fail 0; oxlint 0 errors (warnings unchanged: core 64, social 36, timeline 9, tree 2, workflow 15, interop 1, live-e2e 0);arch-docs: OK (4 changed file(s));ok 10 patches apply cleanly in order+ok core/ equals ds-bot/ + patches/ds-bot/;changelog.d/valid (16 fragments).Not run: CI checks at this tip — the repository's check queue has been blocked since 03:54Z, so nothing was waited on and nothing was rerun; the local
npm run verifyabove is the gate until CI recovers. Also not run: the smoke test (nocore/index.jsorcore/src/host/change), thepackagejob's typecheck / actionlint, and no live instance or browser (the dock is covered by the client bundle test).Checkpoint
{ "pr": 1639, "claim": "Dev3", "issue": 1638, "summary": "A room's own pending approval card could be drawn as the no-button waiting card while the dock's two 5 s lists were one poll behind. The room object already carries the same pendingApprovals array pendingCount counts, so the dock now runs that list through its own drawing rule: a card it would draw keeps it quiet for that beat, and a card it never draws (decided on its own bubble or in the Bot's chat) keeps the waiting line. The dead thisRoomHeld guard is gone.", "files": ["plugins/social/client.js", "plugins/social/test/approval-dock.test.js", "docs/subsystems/social.md", "changelog.d/1638-approval-dock-room-card.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=1ed614a7fd npm run verify", "result": "green", "at": "5e5356a04824243a90e0884ac6b39612adcc804d" }, "next": "none — ready for review" }Risk:
plugins/social/client.js— the tier is computed byrisk-gatefrom the diff.