Repository navigation
Keep a group's fixed rules in its standing section, not in every turn (#894) - #897
Conversation
…ThinkFlowLab#894) Every group turn re-sent the members line, @name calls, the full notice, the admin/lead notes, the face roster and routing rule and the reply rule, and the member's group Session stored each copy. They are now the bot-room system-prompt section, which DSH renders on every step and after a checkpoint; a turn carries the round and says only when the notice changed. Refs ThinkFlowLab#796
|
Verdict: PASS at 5f59985 Ran:
Not run: smoke bench ( Findings: no actionable findings.
Security fence / display-name: room text in the section is the local notice (set only through Disclosure: the same executor session that wrote this PR ran this review (owner's instruction to fix these together; |
…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.
Written-by: Dev
Fixes #894
Refs #796
A member's group Session got the group's fixed text in every turn's message — members line,
@Namecalls, the full notice, admin / lead / member notes, the face roster and the face routing rule, the reply rule — and stored each copy (groups.jsroomPrompt→room-participant.js:66rt.deliver). They are now thebot-roomsystem-prompt section of that Session (ctx.systemPrompt.section, asprompt.js/memory.jsalready do). DSH renders sections on every model step and re-projects them after a checkpoint or in a new part, so every rule stays in front of the model. A turn now carries only the round (earlier lines, this round, references, who is working, speaking order, position), a short routing note on a routed face line (this line: to everyone / the lead's / "Default to [PASS]", who spoke last, who already answered), and a one-line reply cue. When the notice changed since that member's last turn, the turn says so in one line (room.noticeSeen, a 16-hex SHA-256 mark). Own chats and the relay Session render no section, so their prompts are byte-identical to before.Why not "inject at start + on change + after checkpoint": not needed — the section is the standing mechanism. Why not DSH runtime context: its
runtime-contextuser messages are not a no-request kind inturn-origin.js, so every group turn would turnother(standing approvals, the origin a turn passes on). Peer names in the face roster were already cleaned byfaceRoster; the "other owners' Bots" list now goes throughcleanNametoo, since it moved into the system prompt.Token measurement (mock model, same workload before/after)
c90af02A member that spoke 20 times in that group carries ~28.7k fewer chars (~7.2k tokens) of history, re-read on every later call. Trade-off: on DeepSeek a changed system prompt is appended once in full (DSH
in-history), so an edit to the notice / members costs one system-prompt copy instead of one fixed block per turn; it pays back after roughly 10–20 member turns per edit.Verification at
5f59985b733828f301f4bf1ef9243b2470f522ccTZ=UTC ARCH_DOCS_BASE=origin/main npm run verify→ pass (core 1329/1329, plugins 391 + 125 + 125 + 332 + 37, arch-docs OK, patches check OK).core/test/rounds.test.jsfail onc90af02with only the test file copied in, pass here.node .github/scripts/changelog.mjs check→ ok. Lint warnings: 43 at base, 43 here.Not run: smoke bench (
dev/bench.sh, no DeepSeek Harness checkout on this box), Desktop, a real model.Docs:
docs/subsystems/core-host.md§ Group standing section (#894). Changelog:changelog.d/894-room-standing-rules.changed.md.