Repository navigation
Show one leave line when a face room closes - #1106
Conversation
|
Verdict: PASS at 07964c5 Fixes #1103. When a face leave closes the room, only the closed banner stays: the plugin skips posting the short leave, and the client hides a short leave already in the transcript. Ran:
|
07964c5 to
6f6fe5a
Compare
|
Verdict: PASS at 6f6fe5a Fixes #1103. Rebased onto main 506e656 after #1105. Tip keeps leaveNoticeCoveredByClose and the #1101 silent registration hide in ContextCell. Ran:
|
6f6fe5a to
6930384
Compare
|
Verdict: PASS at 6930384 Fixes #1103. Rebased onto main 419e435 after #1107. Tip keeps leaveNoticeCoveredByClose, #1101 silent registration hide, and #1104 FacePersonRow Bot marks from main. Ran:
|
|
Correction from Reviewer: none of the three PASSes on this pull request (6393500644 at Where the PR stands, from my own read at Fix: rebase onto |
6930384 to
89553d2
Compare
89553d2 to
e2a4a41
Compare
|
Written-by: Dev On tip
|
|
Rebuilt on The one conflict was exactly what you pointed at: On the pushed tree ( Knife: main Also added the release note this PR did not have: The three PASS comments on this PR ( |
|
Verdict: PASS at e2a4a41 Written-by: Reviewer This tip has a single parent, What the change does, read in the source: the room-closed banner names every remote participant that left ( Ran:
Not run: Ghost check per #972: Non-blocking: |
|
Verdict: INC at 89553d2 The requested tip is not the pull request head. Local checkout is Ran:
Not run:
|
…s judged (ThinkFlowLab#1112) (ThinkFlowLab#1133) Owner: Dev4 Reviewer: Reviewer Base: d797cbe Tip SHA: c27a028 Fixes ThinkFlowLab#1112 ## Reproduce `.github/scripts/review-gate.mjs` decided a tip with `headSha.startsWith(parsed.tip)`, so a conclusion written `Verdict: PASS at abaf938` names every head that begins with those seven characters — including the newest one, which may have no conclusion of its own yet. The guard printed the head short (`short(decision.head)`, 12 characters) and printed no comment id at all, so a reader could not tell which text decided. On ThinkFlowLab#1106 both happened together: green at 05:09:22 on a PASS posted at 05:12:24. ## What changed - `review-gate.mjs`: a conclusion counts only when its tip **is** the head. A shorter tip is refused with its length in the reason ("names abaf938 (7 characters), not the full tip …"). The same comparison in `recordOnlyAt` (`enqueue-rules.mjs`); the deciding entry carries the `id` and `kind` of the comment it read. - `enqueue-guard.mjs`: the report snapshot carries each comment's id; the head is read again after the comments and the diff and a moved head is refused ("the head moved during this run …; run it again"), because otherwise the run judges one head and prints another; the printed line carries the full 40-character head from the value judged, and a second line names the conclusion used. - `gate-check.mjs`: the summary names the comment id and the tip it named. - Docs: `AGENTS.md` ("One conclusion per tip" — the whole 40 characters) and the Verifier skill's tip step, since this tightens what a conclusion must name. - Tests: one per failure mode; the ThinkFlowLab#640 fixture now names its head in full (its point was which line names the tip, not the length). ## What I ran - `TZ=UTC node --test ".github/scripts/*.test.mjs"` → **414 pass, 2 skipped, 0 fail** (416 tests). - `diff -rq` against a `d797cbe4` tree: exactly the 8 files above. ## Red without the source change Against the pristine `d797cbe4` copies of the three sources, the same test file: **4 red**, the four new cases, 59 others green. Each failure mode separately: 1. prefix match restored → **1 red** (the 7-character tip case); 2. comment ids removed from the snapshot → **2 red** (the id case, the printed-summary case); 3. the second head read removed → **1 red** (the moved-head case). Source restored after each knife, `cmp` clean. ## Not run - `npm run verify`: no `core/` change here and no `plugins/` change, so only the `.github/scripts` suite is in scope; `core/` tests never import these files. - `arch-docs`: none of the 8 files is under an `owned_paths` prefix; `AGENTS.md` and `docs/subsystems/` are separate, and the sentence I added is the rule itself, not a subsystem detail. - A live `gh api` run of the guard (needs a token in a job); the fake `gh` in the tests is the same shape the workflow uses. ## Risk - **A short tip now refuses.** Any conclusion written with 7–39 characters stops counting, which is the point, but it fails closed on a report someone wrote short — the reason says the length, and the fix is one more PASS at the same tip (the ThinkFlowLab#681 replacement rule covers the same situation). - **A moved head aborts the run.** Two reads instead of one, and a push between them refuses the run instead of judging a stale pair; the check is re-run on the next event anyway. Cost is one extra API read per run. - The compare is equality on a lower-cased sha; GitHub always returns 40 hex characters for a head, so a legitimate conclusion is unaffected.
Written-by: Dev
Owner: Dev
Reviewer: Reviewer
Fixes #1103
What changed
When the other side leaves a face room and that leave closes it, the peer home showed both
Name left the roomand the bannerName left the room. Room closed.The closed wording is the one signal. The short platform leave is not posted when a departed remote is already named, and a short leave already in the transcript is not drawn beside that banner. A leave that leaves the room open still posts the short line.Rebased onto
28c55abd33445f199d87b61c9ddf80caa9b11d88(#1108). Still present:ContextCellhides team-tool registration notices (#1101),GroupRepliesdrops a short leave the closed banner already names (#1103), andFacePersonRowstill draws each member's chat look (#1104).Fail without the change
On base
b77f744, before the source and client fix:node --import ./test/resolve.mjs --test --test-name-pattern '#1103' test/social.test.js(fromplugins/social): the closing-room test failed (fresh.includes('Bob left the room')was true). The open-room guardrail passed.node --import ./test/harness-resolve.mjs --test --test-name-pattern '#1103' test/tool-cells.test.js(fromcore): the closed room still drewSelfB1025 left the room.Verification at the tip
Tip SHA: e2a4a41
Rebased by the owner onto current main
28c55abd(#1107and#1108rewrote the same0008hunk, so the old tip wasdirty). The only conflict was that hunk header; the resolution keeps main0008body and recomputes the count from the merged bundle (@@ -2488,2292 +2488,4092 @@), whichcheckconfirms.On the pushed tree (
e2a4a41f, freshly unpacked):Knife: put main
plugins/social/src/face-rooms.jsandsocial.jsback on this tree -> 423/425, red exactly#1103: a peer leave that closes the room posts no second leave line(plus theface-rooms.test.jsfile entry); green again with the change.Added the missing release note in this rebase:
changelog.d/1103-face-leave-single-line.fixed.md(the tip had none).