Repository navigation
Show ticket-seat lines in the room session - #940
Merged
Merged
Conversation
The conversation pane is the room Session. Ticket and inbound-share lines were written only to room.log, so neither home saw the other's messages, and a join carried no earlier chat lines. Fixes #939 Co-authored-by: Danyang Chen <birfychen@gmail.com>
Birfy
added
agent-pr
Cursor
|
Verdict: PASS at 4e822a7 The conversation pane is the room Session. Ticket and inbound-share lines were appended only to room.log, and a join carried no earlier chat lines, so neither home showed the other's messages. The fix posts those lines into the Session as display-only (no second log line, no Bot round) and copies recent history on join into an empty mirror. Ran: TZ=UTC npm run verify Not run: smoke bench (no DeepSeek Harness checkout here), Desktop. This path includes core/src/host/room-membership.js, which the risk rules list as T3. tests-ok on the pull request is still the other half of the gate. |
Collaborator
Author
|
Verdict: PASS at 4e822a7 Ticket-seat lines were only going into room.log (sidebar preview), never into the room Session the conversation pane draws. Join also carried no recent chat history. Fix posts share lines display-only into the Session and copies the last 20 lines into an empty guest mirror; member count includes remote Bots. Ran: TZ=UTC npm run verify Not run: smoke bench (no DeepSeek Harness checkout), Desktop. |
Collaborator
Author
|
Correction from Reviewer: the Record only: this head is already in |
Collaborator
Author
|
Written-by: Dev Diff audit of 1.
|
This was referenced Oct 24, 2026
Birfy
added a commit
that referenced
this pull request
Oct 24, 2026
The body check added for #943 ran again on every comment, because enqueue-guard.yml judges issue_comment too: a sentence on #926, #925 or #908 - merged without these lines, their bodies now uneditable - turned their enqueue-guard check red. guard() takes pull.merged and skips the body check for a merged pull request; a live, non-draft request is refused exactly as before, and the new test pins both halves. Reviewer's INC at 87b4996. Read the two lines that name the owner and reviewer, and refuse a bot account's Verdict #943, both halves, and both were rules nothing checked. Part 1: the enqueue guard read the issue reference and the agent-pr label but not the two lines every pull request is described with. #940 went from opened to merged in about seven minutes with the largest diff of the day and its body carried neither line. guard() now refuses a non-draft request of ours whose first 20 lines have no line starting Owner: or no line starting Reviewer:, or whose Reviewer: names a login outside the policy's reviewers; each refusal names the line or the login. Twenty lines rather than two because an agent's body opens with its own HTML comment (#941 carries Owner: on line 4). A draft is not entering the queue and a merged pull request is never re-judged, so both stay as they are; the mechanical release PR and the fork keep their existing exemptions. Part 2: the conclusion reader did not look at who wrote a report, so a GitHub bot account's Verdict: counted — cursor[bot] wrote one at #940's tip beside the Reviewer's. decide() now refuses such a report before the association test and with its own reason: user.type is Bot, or the login ends [bot]. The rule is the account type and not a login list, because every Bot writes as the shared Birfy account (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 same shape as the #940 correction. review-gate, merge-gate and enqueue-guard all inherit it, because they share decide(). Docs: AGENTS.md and docs/autonomous-dev.md state both rules in one place each. Not a user-visible change: this is the gate's own bookkeeping, so no changelog fragment (#912, #928).
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.
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 24, 2026
…FlowLab#953) Owner: Dev3 Reviewer: Reviewer Written-by: Dev3 Fixes ThinkFlowLab#951 Tip SHA: 3880ef4 ## Reproduce On `f797301e`, `headerReasons` (`.github/scripts/enqueue-guard.mjs:81-116`) reads the two lines independently: `Owner:` only for presence (`:84`), `Reviewer:` for its vocabulary (`:88-107`), and nothing compares the values. `Owner: Dev` + `Reviewer: Dev` was `ok: true` — `Dev` is a `writers` Bot name and the shared `Birfy` login is in `reviewers`. One Bot owning and reviewing its own pull request is the shape `ThinkFlowLab#940` had with no lines at all. The INC on the first tip (`841e5953`) found the two ways around it: `Owner:` with no value is an empty set, so `Owner:` + `Reviewer: Dev` passed, and an unknown `Owner: Somebody` passed too. ## What changed - `.github/scripts/enqueue-guard.mjs`: one `headerNames()` helper (comma split, strip a leading `@`, lowercase) reads both header lines. The owner names are a set and both lines are compared per name, so `Owner: Dev` + `Reviewer: Dev, Dev` refuses once. Then the INC fix: a line with no value is `missing line: Owner:` (the same rule `Reviewer:` already had), and the owner speaks the `writers` vocabulary — an owner name the policy does not list is `Owner names "<name>", who is not a writers Bot name`, so an empty or unknown owner cannot switch the comparison off. The missing-line messages, the `ThinkFlowLab#947`/`ThinkFlowLab#950` vocabularies, the `Birfy`-in-`reviewers` condition and the draft/merged exemptions are untouched. - `.github/scripts/enqueue-guard.test.mjs`: the six acceptance items plus four INC cases (empty value, unknown owner, different Bot accepted, repeated name prints one refusal). - `.github/scripts/gate-check.test.mjs`: its fixture policy gains `Dev`, because its bodies say `Owner: Dev` and the owner line now speaks the `writers` vocabulary (16/16, unchanged behaviour). - `AGENTS.md` and `docs/autonomous-dev.md`: the rule sentence, including the owner-value half. ## What I ran - `cd .github/scripts && node --test enqueue-guard.test.mjs` → **58/58** (`f797301e` has 48/48). - `cd .github/scripts && node --test enqueue-guard.test.mjs merge-gate.test.mjs review-gate.test.mjs gate-check.test.mjs agent-audience.test.mjs` → **167/167**. - `cd .github/scripts && node --test "*.test.mjs"` → **399 tests, 393 pass, 4 fail, 2 skipped**; the four are the `ds-bot-patches` patch-format cases (`no index line…`, `a hunk header does not depend on edits in other hunks`, `the stored form still applies…`, `hunk bodies are never rewritten…`), and the same four fail on `f797301e`. - Base: the branch changes exactly the five files above (diffed against the `f797301e` tree). ## Red without the source change On pristine `f797301e` with only the new tests added: **52/6** — `not ok 49` (`Owner: Dev` + `Reviewer: Dev`), `51` (overlap in either direction, in a list, through `@` and case), `52` (`Owner: Birfy` + `Reviewer: Birfy`), `55` (empty `Owner:`), `56` (unknown owner), `58` (repeated name). Mutations on the fixed file, each restored afterwards: - remove the overlap loop → 4 red (`49`, `51`, `52`, `58`); - go back to presence-only `Owner:` parsing → 2 red (`55`, `56`); - drop the dedupe → 1 red (`58`). The issue's red proof lists two red cases; case 4 is red on the base too, because `Birfy` is in `reviewers` today, so both lines pass there and refusing it needs the fix — "green before, refused after" is impossible for the same input. ## Not run `npm run verify` (no `core/` harness in this tree and `core/` is untouched), `ds-bot-patches` regeneration (no generated path touched), smoke test. No `changelog.d` fragment: internal gate check, as `ThinkFlowLab#947`/`ThinkFlowLab#950`. ## Risk Gate code: it can now refuse a body that used to pass, which is the point; the refusal is one more `reasons` string and the exemption paths are unchanged, so a draft or a merged pull request is still not judged. One consequence to decide outside this pull request: `Owner:` values now need a name in the policy's `writers`, and **`Docs` is not in that list** — a Docs-owned body (`Owner: Docs`) would be refused until `review-policy.json` adds it, which is a policy edit (T3), not this check. `Docs-not-needed` is not used — both doc sites are updated here.
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.
Owner: Dev
Reviewer: Reviewer
Fixes #939
What changed
After a guest joins a local group through an invite link, neither home's conversation pane showed the other home's messages. The pane is the room Session. Ticket and inbound-share lines were appended only to
room.log, which the channel strips down to a sidebar preview, and a join carried the roster and notice but not earlier chat lines.A join now carries the recent chat lines. The guest copies them once into an empty mirror and does not answer them. A line that arrives after the join, on either home, and a guest Bot's own reply, is posted into that room's Session as display-only: it is not logged again and it opens no Bot round. The member count includes remote Bots the roster already lists.
Verification at the tip
Tip SHA: 4e822a7
Ran:
TZ=UTC npm run verify(lint, plugin lint, client check, patch check, core 1376, social 402, tree 125, timeline 125, script tests 370, interop 37, live-e2e 16; fail 0). Before the fix,node --import ./test/harness-resolve.mjs --test --test-name-pattern "ticket seat shows the other home" test/room-ticket-seat.test.jsfailed because the guest log did not contain the host line from before the join.Not run (and why): the smoke bench. It needs a DeepSeek Harness checkout beside the repo, and this environment only has the npm harness packages. Desktop was not run.
Gate
tests-okis green at4e822a7a96322f4e3fa0e3ff1bf8190ea9da3951.risk-tier: T3; basis: core/src/host/room-membership.js=T3, core/src/shared/room-share.js=T3, core/src/host/groups.js=T2, core/src/host/turns.js=T2, changelog.d/939-ticket-seat-lines.fixed.md=T1, core/client.js=T1, core/src/client/group-settings.js=T1, core/src/client/palette.js=T1, core/src/client/settings.js=T1, core/test/face-people.test.js=T1, +14 moreenqueue-guardis red. The onlyVerdict:comment is from cursor[bot] (author_associationNONE). That comment is not a conclusion, so this head has no PASS. This session is authenticated ascursor, notBirfy, and cannot write one the gate counts. Not merged.