Repository navigation
Promote a room-only identity for a group link, and say joined only when the room is here (#899) - #900
Merged
Merged
Conversation
…en the room is here (ThinkFlowLab#899) A Bot that used a face-to-face room keeps a room-only Bot Social identity: the host does not poll it and the guest cannot send the knock from it, so the claim landed and nobody was seated, and the dialog then said 'already joined'. Fixes ThinkFlowLab#899.
Collaborator
Author
|
Verdict: PASS at 6ae1491 Ran:
Not run: full Findings: no actionable findings.
Disclosure: the same executor session that wrote this PR ran this review (owner asked for one focused review, speed first; |
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 24, 2026
…unt's Verdict (ThinkFlowLab#944) Owner: Dev Reviewer: Reviewer Fixes ThinkFlowLab#943 Written-by: Dev ## What changed Both halves of ThinkFlowLab#943, each a rule the pipeline described and nothing read. **Part 1 — the two lines, before the queue.** `guard()` (`.github/scripts/enqueue-guard.mjs`) now refuses a **non-draft** request on a branch this repository owns when the first 20 lines of its body carry no line starting `Owner:` or no line starting `Reviewer:`, or when `Reviewer:` names a login outside the policy's `reviewers`. Each refusal names the line, not just the defect: `missing line: Owner:`, `missing line: Reviewer:`, `Reviewer names "x", who is not in reviewers`. Twenty lines rather than two because an agent body opens with its own HTML comment — the open ThinkFlowLab#941 carries `Owner:` on line 4 — and a top-two check would refuse every such pull request. A `Reviewer:` value is one login, comma-separated, `@` and any case allowed; a `Reviewer:` with no value names nobody and is refused as the missing line. A **draft** is not entering the queue and is not refused, and nothing older is ever re-read: ThinkFlowLab#940's merged record stays exactly as it merged. The mechanical release PR and the fork keep the exemptions they already had. **Part 2 — a bot account writes no conclusion.** `decide()` (`.github/scripts/review-gate.mjs`) refuses a report authored by a GitHub bot account — `user.type === 'Bot'`, or a login ending `[bot]` — before the association test, with its own reason naming the account, so the log says which defect fired. The report still joins `judged` with `counts: false`. The rule is the **account type, not a login list**: every Bot writes as the shared `Birfy` account, which is a `User`, and a list containing it would void the whole pipeline. What voids such a report is the real Reviewer's `Correction from Reviewer:` — the shape the Reviewer already used on ThinkFlowLab#940 ([6381646389](ThinkFlowLab#940 (comment))). `review-gate`, `merge-gate` and `enqueue-guard` all inherit it because they share `decide()`; the two API readers now carry `user.type` alongside the association. Docs: `AGENTS.md` (Merge gates: the queue bullet and `One conclusion per tip`) and `docs/autonomous-dev.md` (`§ The reviewed tip` rule 6, and the enqueue-guard sentence) state each rule once. Not a user-visible change — the gate's own bookkeeping, same precedent as ThinkFlowLab#912 and ThinkFlowLab#928 — so no `changelog.d/` fragment. ## INC at `87b49961` answered Reviewer was right, and the line in the first version of this body was false: "a merged pull request is never re-judged". `enqueue-guard.yml` judges `issue_comment` too, so **any** sentence on a merged pull request runs this check again and the two header reasons fired on a body nobody can edit any more — the live list of merged pull requests without these lines is ThinkFlowLab#926, ThinkFlowLab#925, ThinkFlowLab#908, ThinkFlowLab#901, ThinkFlowLab#900, ThinkFlowLab#898, ThinkFlowLab#897 (ThinkFlowLab#940 escaped only because I edited its body afterwards). The fix is the one Reviewer named: `decidePull` already reads `pull.merged`, so `guard()` takes it and the body check is `if (!draft && !merged)`. The new test pins both halves — `ThinkFlowLab#943 INC: a merged pull request is not re-judged when a comment arrives`: the same body (no lines, a PASS at the tip) is green when `merged`, and still refused with both `missing line:` reasons when it is an open non-draft request. Mutation: dropping `&& !merged` reds exactly that test. Checked against the real bodies, through the branch's `guard()`: ```text ThinkFlowLab#926 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer: ThinkFlowLab#925 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer: ThinkFlowLab#908 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer: ThinkFlowLab#897 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer: ``` `AGENTS.md` now says "a **merged** pull request is never re-judged: this check also runs on every comment, and history is not a request", and `docs/autonomous-dev.md` says the same next to the enqueue-guard sentence. ## Evidence Red first, on the pristine `c7286584` with only the new tests added (11 red, 3 controls): `.github/scripts/enqueue-guard.test.mjs` 36/7 — no `Owner:`, no `Reviewer:`, `Reviewer:` outside `reviewers`, the lines after the 20th, a `Reviewer:` with no value, a bot-account `Verdict:`, and the INC case below; `.github/scripts/review-gate.test.mjs` 45/3; `.github/scripts/merge-gate.test.mjs` 35/1. The three part-1 tests that stay green there are the controls that catch over-refusal: an `@`/mixed-case listed login, the two lines at line 4-5 (ThinkFlowLab#941's shape), and a draft. Seven mutations, each red on its own test and restored byte-identical: drop `if (!draft)` → the draft control; `slice(0, 20)` → `slice(0, 2)` → the line-4 test; drop the membership loop → the unlisted login; drop the `Owner:` check; drop the `[bot]` half of the account test; allow an empty `Reviewer:` value; drop the bot check in `decide`. Gate 4 for part 2 replays ThinkFlowLab#940: `cursor[bot]`'s report at that tip is `counts: false` with the bot-account reason, while `Birfy`'s report at the same tip still counts — asserted in `review-gate.test.mjs` and again through `merge-gate`'s queue entry. At tip `23f14dbaaba6c283f2acca7259cfa99575f0ef74` (the branch head, no commit after it): ```text $ node --test ".github/scripts/*.test.mjs" # tests 383 / pass 377 / fail 4 / skipped 2 ``` The four failures are the `ds-bot-patches` format tests, which shell out to `git diff`/`git apply`; this box has no `git` (the repo's own note in `.github/scripts/fixtures/review-reports-660-663.json` says the same four fail on main without it). Per file: `enqueue-guard` 42/42, `review-gate` 48/48, `merge-gate` 36/36, `gate-check` 16/16 — `gate-check`'s fixture pull bodies now carry the two lines, because the guard it drives reads them. `node .github/scripts/changelog.mjs check` → 54 fragments well formed; `testing-layers.mjs --check` → 350 files in 15 workflows, all classified; `self-test-catalog.mjs --check` → page and catalog agree; `oxlint` on the six touched JS files → 0 errors (one pre-existing warning in `gate-check.test.mjs:101`, untouched). Not run: `npm run verify` end to end (no `node_modules`/harness on this box, and `arch-docs` needs `git log`); no owned subsystem file changed, so `arch-docs` has nothing to pair here. CI's `tests-ok` and `package` cover both. ## Risk T3 by surface: `.github/scripts/` and the two policy documents. It changes no product code path, no `core/`, no `patches/`, so the patch round-trip and the client bundle are untouched; what it can do is refuse a pull request that used to reach the queue, which is the point of it. No bypass, no label, no history rewritten. ## Scope Eight files: the two source scripts, four test files, the two docs. No `core/`, no `patches/`, no generated file, so the patch round-trip is untouched. The check is enqueue-time only — it is deliberately not added to `review-gate`'s or `judge`'s conclusion tests, which read every report on every pull request and would refuse reports on pull requests that never had the two lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written-by: Dev
Fixes #899
A Bot that used a face-to-face room keeps a room-only Bot Social identity.
mintRoomInviteandclaimTicket(core/src/host/room-membership.js) asked for an identity only when a Bot had no handle, so a room-only one stayed as it was:sendTicketShare(needs an active member): Chief has no Bot Social identity yet;pollOncepolls only active members, so the knock was never read and nobody was seated;plugins/social/client.jspreviewLocal) then said You already joined this group with that link for anyclaimed.Fix: a room-only identity counts as none for a link (
activeHandleOf), soidentifypromotes it at mint and at Join;serveInvitesrunsrt.identifyInviteBotsonce so links minted earlier on a room-only Bot are read after an update.room-invite-previewaddsjoined(the room that ticket seated is here); the dialog says "already joined" only then, otherwise Waiting for to let you in… with the card kept, so Join claims again (same Bots = idempotent on Bot Social) and knocks again.Reproduced on main 9f4aa52 + Bot Social 929d390 (byte-identical to what prod serves) with two fresh homes: both failures above.
Verification at
6ae149150f36e5557f05b239c88de1a7348ff628core:node --test test/room-ticket-seat.test.js test/remote-room.test.js test/room-membership.test.js test/authz.test.js→ 99/99.plugins/social:TZ=UTC npm test→ 395/395.#899tests incore/test/room-ticket-seat.test.jsand the updated dialog test inplugins/social/test/social.test.jsfail with the base sources (fail 2 / fail 1), pass here.ARCH_DOCS_BASE=origin/main npm run lint(core) → arch-docs OK, 43 warnings (same as base);node scripts/lint-plugins.mjsok;build-client --checkok;ds-bot-patches checkok;changelog.mjs checkok.Not run: full
npm run verify(CI runs it), smoke bench, Desktop.