Repository navigation
Collapse a repeated Bot-to-Bot line into one turn (#641) - #680
Merged
Merged
Conversation
Collaborator
Author
|
Verdict: PASS at 7fde660 Reviewed at this tip only. All 9 checks are What I checked
Non-blocking — please fix in a follow-up; this does not need another round
Risk: |
Birfy
added a commit
to Birfy/FlyBot
that referenced
this pull request
Oct 23, 2026
…wLab#694) Written-by: Dev Adds `.github/scripts/audit-merges.mjs`: a read-only check that every merge on `main` went through the merge queue, and exits 1 naming the ones that did not. Fixes ThinkFlowLab#689 Tip SHA: 2372531 ## Reproduce `ThinkFlowLab#650` and `ThinkFlowLab#680` are build commits whose `Merge gate` run had failed, and `main` was pushed to them anyway — the runs API, live: ```text ThinkFlowLab#680 run 39730969259 Merge gate failure 2026-10-22T05:57:11Z head 6b9820b branch gh-readonly-queue/main/pr-680-8e80237b main moved to 6b9820b at 05:58:58Z, merged_by Birfy ThinkFlowLab#650 run 39653640861 Merge gate failure 2026-10-21T18:32:27Z head 6438cb2 branch gh-readonly-queue/main/pr-650-fe79de66 main moved to 6438cb2 at 18:34:05Z, merged_by Birfy ThinkFlowLab#672 run 39720497176 Merge gate success 2026-10-22T04:06:23Z head 93e5234 ThinkFlowLab#674 run 39722858008 Merge gate success 2026-10-22T04:31:32Z head 9393d55 ``` Nothing in the repository tells the two shapes apart: the only `merge_group` record of the landed commit is refused, and no check looks at what landed. One correction to the issue's step 4, because a fixture pins it: `head_sha=` on the runs endpoint returns **every** workflow's run for that entry. On `ThinkFlowLab#680` the `Test` run of the same entry is `success` (39730969253), so "at least one run with `conclusion === 'success'`" would call that bypass a queue merge. The check therefore matches the **`Merge gate`** workflow (`path === .github/workflows/merge-gate.yml`), and a green `Test` run proves nothing about the gate. ## What changed `.github/scripts/audit-merges.mjs` (new, ~200 lines, no new dependency), its cases and four fixtures: - `auditCommit({sha, pulls, runs})` — one commit in, one finding or `null` out. No pull request → `direct commit: <sha>`. A successful `Merge gate` run whose `head_sha` is this commit → queue merge. Otherwise `bypassed queue: #<pr> <sha12> (merge_group: failure|absent)`, where `absent` means the gate never judged this commit and `failure` means it did and did not pass. - `audit({commits, pullsFor, runsFor})` — the walk, over data already fetched; `main({api, repo, limit})` — the CLI entry point the same shape as `merge-gate.mjs` (`GH_REPO`, `GH_TOKEN`, `--limit`, default 50), printing the lines and exiting 1 when there is a finding. - Read-only: no comment, label, merge or ref push. It walks `/commits?sha=main`, `/commits/{sha}/pulls`, `/actions/runs?event=merge_group&head_sha={sha}`. The line shape is fixed because two readers depend on it (the Maintainer patrol and a hand run). A direct push is reported on its own line and is not counted as a bypass. ## What I ran At this tip, on the extracted tree: - `TZ=UTC node --test .github/scripts/audit-merges.test.mjs` → **9/9** - `TZ=UTC node --test ".github/scripts/*.test.mjs"` → **173/173** Against the real repository, through this script's own `main()` with its `api` injected over the same REST paths `gh api` uses (`gh` is not installed on this box), at main `b5de55d0`, `--limit 40`: ```text bypassed queue: ThinkFlowLab#680 6b9820b (merge_group: failure) bypassed queue: ThinkFlowLab#636 8e80237 (merge_group: failure) bypassed queue: ThinkFlowLab#650 6438cb2 (merge_group: failure) ... audit-merges: 40 commits on main, 31 bypassed the queue, 0 direct exit code: 1 ``` `ThinkFlowLab#672` and `ThinkFlowLab#674` produce no line, which is the acceptance condition. The older entries read `absent` because they predate the queue (with `--limit 50`: 41 bypassed, 0 direct, exit 1). ## Red without the source change Three mutations of the guard, each reverting only `audit-merges.mjs`: - the success test becomes "any `merge_group` run exists on the commit" (the issue's literal step 4) → **5 of 9 red**, including the `ThinkFlowLab#680` fixture: the green `Test` run is read as the gate passing. - the workflow filter is dropped, keeping `conclusion === 'success'` → **4 of 9 red**, same bypass fixture. - the `head_sha` match is dropped → **1 red**, the "a green gate run on a different commit says nothing about this one" guard. Restored → 9/9. ## Not run - CI at this tip has not finished yet; no workflow runs this script (wiring it into a routine is out of scope in the issue). - `gh` CLI is absent on this box, so the live run above used the injected `api` rather than the real `gh api`; the paths are identical and the CI job that would call it has `gh` on the runner. - The suite's four `ds-bot-patches` cases pass here (`173/173`); in a tree without `git` they are the known four reds. ## Risk **T3** — `.github/**`, per `.github/agent/risk-rules.json`; needs a PASS at this tip and a maintainer approve. Docs-not-needed: a read-only `.github/scripts` check and its cases; no subsystem doc owns `.github/`.
Birfy
added a commit
to Birfy/FlyBot
that referenced
this pull request
Oct 23, 2026
… green run (ThinkFlowLab#697) (ThinkFlowLab#706) Written-by: Dev3 Fixes ThinkFlowLab#697 Tip SHA: 3e8ea72 ## Reproduce `audit-merges.mjs` is red on every historical bypass, and nothing runs it. Live, against `main` at `bafe9bad` with the script this branch changes, `--limit 40`: ``` $ node .github/scripts/audit-merges.mjs --limit 40 # today's behaviour bypassed queue: ThinkFlowLab#680 6b9820b (merge_group: failure) bypassed queue: ThinkFlowLab#636 8e80237 (merge_group: failure) … 24 more lines … audit-merges: 40 commits on main, 26 bypassed the queue, 0 direct exit 1 ``` 26 of the last 40 commits, so the check is red on every run and therefore unreadable; and no workflow and no document named the script, so nobody saw it. ## What changed - `.github/scripts/audit-merges.mjs`: `--since <sha|ISO date>` (a sha is resolved to its committer date through `GET /commits/{sha}`) and `--baseline`. A finding at or before the watermark prints as `earlier bypass: …` and does not fail the run; a finding after it prints as `bypassed queue: …` and exits 1; `--baseline` treats every finding as earlier, which is what a first run uses before there is a previous run to point at. `audit()` carries each commit's committer date, `classify()` applies the watermark, and `exitCodeFor()` counts only findings that are not earlier. With neither flag the behaviour is exactly today's. - `.github/workflows/audit-merges.yml` (new): daily at 04:41 UTC plus `workflow_dispatch`, `permissions: contents/actions/pull-requests: read`, no write scope at all, pinned actions, script taken from `main`. Its watermark is the `head_sha` of its own previous **successful** run (`runs?status=success&per_page=1`), so it commits no watermark file; when that lookup is empty it runs `--baseline`, so the first green run *is* the baseline. - `.github/scripts/audit-merges.test.mjs` + `fixtures/audit-merges/since.json`: nine new cases — all-older exits 0 with every line as `earlier bypass`, one newer asks for exit 1 and leaves the other earlier, a sha watermark resolves to its committer date and is itself older, `--baseline` never fails, no flag keeps today's behaviour, a date needs no commit lookup, a bad `--since` refuses, a finding with no date is not assumed old, and the workflow's shape (read-only, daily, previous-run watermark, pinned actions). - `docs/autonomous-dev.md`, Merge plumbing: one paragraph naming the patrol and saying what a red run means. One deliberate deviation from the issue's wording: the walk is **not** narrowed with `GET /commits?…&since=`. The acceptance asks for a run where every finding is older to print each one as `earlier bypass:` and exit 0 — a walk that starts at the watermark has no older commit left to print. So the watermark is applied to each commit's committer date and the walk keeps its `--limit`; the API calls are the same three as before, plus one for a sha watermark. ## What I ran ``` $ node --test .github/scripts/audit-merges.test.mjs # this branch 1..18 # pass 18 # fail 0 $ node --test ".github/scripts/*.test.mjs" # clean extraction of main bafe9ba + this branch # tests 182 # pass 178 # fail 4 $ node --test ".github/scripts/*.test.mjs" # the same extraction without this change # tests 173 # pass 169 # fail 4 ``` The 4 failures are the `ds-bot-patches` cases that need `git`, which this box does not have; they are identical on both sides. Everything else is green, including `test-workflow` and the label docs checks. Live against this repository, through the branch's own `main()` (`--limit 40`; `gh` is not installed on this box, so the API answers were pre-fetched and served to `main()` through its injectable `api`): ``` $ … --since cbce2b0 earlier bypass: ThinkFlowLab#680 6b9820b (merge_group: failure) … 25 more … audit-merges: 40 commits on main, 0 bypassed the queue, 0 direct, 26 at or before the watermark audit-merges: watermark 2026-10-22T07:29:05.000Z (from cbce2b0…) exit 0 $ … --since 93e5234 bypassed queue: ThinkFlowLab#680 6b9820b (merge_group: failure) bypassed queue: ThinkFlowLab#636 8e80237 (merge_group: failure) bypassed queue: ThinkFlowLab#664 7223a1a (merge_group: failure) earlier bypass: ThinkFlowLab#670 372b064 (merge_group: failure) … 22 more earlier … audit-merges: 40 commits on main, 3 bypassed the queue, 0 direct, 23 at or before the watermark exit 1 ``` So the same history is green when the watermark is the head of the last run and red on exactly the three merges newer than an older watermark — the two halves the issue asks for, on real data. ## Red without the source change ``` $ # variant A: main's audit-merges.mjs + this branch's tests not ok 1 - .github/scripts/audit-merges.test.mjs # imports classify / sinceDate, which main does not export # tests 1 # pass 0 # fail 1 $ # variant B: this branch's script with the classify() call removed (the watermark filter gone) not ok 10 - ThinkFlowLab#697: a watermark after every finding prints them as earlier bypass and exits 0 not ok 11 - ThinkFlowLab#697: one finding after the watermark alone fails the run, the other stays earlier not ok 12 - ThinkFlowLab#697: a sha watermark is resolved through its own committer date, and is itself older not ok 13 - ThinkFlowLab#697: --baseline is the first run: every finding is earlier and nothing fails # tests 18 # pass 14 # fail 4 ``` Exactly the four watermark cases go red; the "no flag keeps today's behaviour" case stays green, so the filter is what those four measure and nothing else moved. ## Not run - The workflow itself: `audit-merges.yml` does not exist on `main` yet, so it cannot be dispatched from this branch (a `workflow_dispatch` run needs the file on the default branch). Its first real run after merge is the `--baseline` one, and its shape is covered by the text test above. - `npm run arch-docs`: `core/scripts/check-arch-docs.mjs` answers `arch-docs: FAIL — cannot diff against origin/main` because this box has no `git`. CI runs it. - A hand run through the real `gh` CLI: no `gh` binary here, hence the injected `api` in the live run above. - No changelog fragment: an internal audit, not user-visible product behavior. ## Risk - The patrol adds a scheduled job: daily at 04:41 UTC, read-only, ~40 commits × 3 API calls, well inside the 1,000/hour budget, and it can be disabled from the Actions tab like `risk-tier.yml`. - The watermark is the previous **successful** run, so a bypass that lands while the patrol is red keeps it red until it is fixed — that is intended (a bypass is a defect), and the fix is an empty one if the bypass was legitimate: a manual `workflow_dispatch` after the fact moves the watermark forward. - `--limit 100` in the workflow bounds the walk: a bypass older than the window is not reported at all. That is the same bound the script always had. - Tier T3 (`.github/**`); no write permission, no secret, no label or comment written by the job.
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.
Fixes #641 — a Bot-to-Bot line that repeats what the same sender already delivered to that Session within the last minute now makes one turn instead of two, and the drop is written to the host log. One host change, its tests, the subsystem doc, one changelog fragment and the regenerated patch series.
Reproduce
deliver()had no memory of what it had just handed a Session, so everymessage_botcall made a turn of its own, even when the text was the same conclusion relayed twice:core/test/repeated-delivery.test.jsreproduces it through the publicmessage_bottool with three Bots and a fake clock: the same text twice from one sender to one recipient produced['steer', 'steer']instead of['steer'].What changed
core/src/host/delivery.jsduplicateDelivery(fromId, sessionId, text)keeps the last minute of Bot-to-Bot text keyed by sender + recipient Session + the text with its whitespace collapsed (DEDUPE_MS = 60_000,dedupeText). A repeat inside the window returnstrue; the map is trimmed and capped at 500 entries (DEDUPE_KEPT), so a long-lived host cannot grow it without bound.deliver()judges a line right after it resolvestargetSessionIdand before the exchange is recorded,firstContactis stamped and the message is handed to the Session. A drop returns the exchange id unchanged, records no exchange, and journalsds-bot: delivery: dedupe <sender> -> <session>throughrt.warn, because a silent drop is how a real report goes missing.fromId === undefined(the user, a group round) is untouched,remote?.handle(a friend's line) is untouched, anddeferred/resumed/retried/resentcarry on a message the Session already has, so dropping one of those would lose it rather than deduplicate it.core/test/repeated-delivery.test.js— four cases: the repeat collapses and is journalled; the same text 61 s later is news again; two senders with identical text are both delivered; the key includes the Session, so another recipient still gets it.core/test/connector-access.test.js— that test relays the literal'hi'several times in a row to check whose request a relay is, which a one-minute dedupe would swallow; each relay now sends its own text, so the origin assertions still test origin.docs/subsystems/core-host.md— the dedupe rule, its key, what it deliberately skips, and the cap.changelog.d/641-repeated-bot-message.fixed.md— the user-visible change.patches/ds-bot/— regenerated (generate, thencheck) becausecore/changed.What I ran
Tip SHA:
7fde6604b22bec9647608ccfff0d85ce614642aa(Node 22.23.3,TZ=UTC, base9393d558):core, full suite: 1216/1216 pass (node --import ./test/harness-resolve.mjs --test --test-concurrency=4 "test/*.test.js", no cancellations).core/test/repeated-delivery.test.json its own: 4/4 pass.core/test/connector-access.test.js+repeated-delivery.test.js: 20/20 pass.node scripts/ds-bot-patches.mjs checkaftergenerate→ pin matchesFei2411/DeepSeek-Bot@2b10c70, 10 patches apply cleanly in order,core/ equals ds-bot/ + patches/ds-bot/.node core/scripts/lint.mjs(withDSH_HARNESS) → 0 errors, 43 pre-existing warnings; it also printsarch-docs: OK (6 changed file(s)).node .github/scripts/changelog.mjs check→ 10 fragments well formed;node --test ".github/scripts/*.test.mjs"→ 153/153.node core/scripts/build-client.mjs --check→ exit 0.git diff --shortstat 9393d558...7fde6604→ 8 files, +360/−7.Red without the source change
With
core/src/host/delivery.jsat main9393d558and the new tests kept:#641: the same conclusion sent twice within a minute makes one turn, and the drop is journalled→ fail,one turn, not two: actual['steer', 'steer'], expected['steer']— the defect itself, one line becoming two turns.Not run
deliver()boundary rather than over the wire.npm run verify(it also runs the three plugin suites; no plugin file is in the diff) and the smoke bench (dev/bench.shneedspnpm, which this box does not have).tests-okat the tip covers the rest.Risk
risk-gate.mjs(core/src/host/delivery.jsis a T3 path; 178 changed lines in 5 files once the generated patch mirror is excluded): it needs a PASS at this tip plus greentests-ok, and the gating agent merges it itself after that.deferred/resumed/retried/resentare never judged.