Repository navigation
Keep children's load out of a tree coordinator's system prompt (#895) - #898
Merged
Merged
Conversation
…FlowLab#895) The Child load line changed the coordinator's tree block at every child turn start and end, so its whole system prompt changed and DSH re-sent all of it on the next step. list_children already reports load, openForMs and stalled live; the coordinator rules now point there in fixed words. Refs ThinkFlowLab#796
…kFlowLab#895) Load only breaks a tie between children whose scope fits, so a part with one fitting child costs no extra tool call. Refs ThinkFlowLab#796
Birfy
force-pushed
the
fix/895-tree-load-line
branch
from
October 23, 2026 17:46
bbc5f0e to
3d4e268
Compare
Collaborator
Author
|
Verdict: PASS at 3d4e268 Ran:
Not run: smoke bench, Desktop, a real-model run. Findings: no actionable findings.
Security fence / display-name: not in scope (no remote text or fencing touched). Disclosure: the same executor session that wrote this PR ran this review (owner's instruction to fix these together; |
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 #895
Refs #796
A tree coordinator's system prompt carried
Child load right now — Dev: working, Test: idle …(#507, #624).plugins/tree/index.jsrewrites the tree block at everyturn.start/turn.endof a tree Bot, so the coordinator's system prompt changed at every child turn boundary (and every minute for a stalled child). On DeepSeek DSH then appends the whole changed system prompt after the history (systemPromptUpdate: "in-history"); on other routes the cached head misses.The load line is gone from the prompt (
notesForonly: renames, roles and other fixed notes still refresh the block, andwriteBlockskips an unchanged block). Coordinator rule 3 now says once, in fixed words: when more than one child fits a part, calllist_childrenbefore sending it, and prefer an idle or working child to a busy one. Load only ever broke a tie between children whose scope fits, so a part with one fitting child costs no extra call.list_childrenalready returns each child'sload,openForMsandstalledlive (#507, #624, #733), so the coordinator reads the same facts, fresher, when it routes.loadLinestays a pure helper inload.js(its own tests keep it); the prompt no longer calls it.Token measurement (plugin test harness, same workload before/after)
c90af02list_childrencall when more than one child fits a part (a few hundred chars)Verification at
3d4e268057209eda3ba85770b6a01abc095405a3(rebased onb54f38a)TZ=UTC ARCH_DOCS_BASE=origin/main npm run verify→ pass (core 1328/1328, plugins 395 + 125 + 125 + 332 + 37 + 16, arch-docs OK, patches check OK).plugins/tree/test/tree.test.js(prompt byte-stable across a child's turn start/end) fails onc90af02, passes here.Not run: smoke bench, Desktop, a real model.
Docs:
docs/subsystems/tree.md§ Runtime load. Changelog:changelog.d/895-tree-load-line.fixed.md.