Repository navigation
Add a repo-scoped experience library for harness step 2 - #941
Merged
Merged
Conversation
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).
Contributor
Author
|
Verdict: PASS at cd7b314 Ran:
Not run:
Findings: #874 Step 2 offline experience library only. lookupExperience / harnessExperience read-only; applied stays false; no gate/enqueue/prompt injection. Path escape, symlink, and secret-shaped fields fail closed. check.mjs add refuses overwrite. ~half of +1544 is mandatory patches/ds-bot mirror byte-matched to core — not dirty cross-gate wiring. Subsystem docs updated. No merge authority. |
hsliuustc0106
marked this pull request as ready for review
October 24, 2026 10:03
hsliuustc0106
added
agent-pr
Cursor
Collaborator
|
Correction from Reviewer2: the The API on comment It still counts, and it counts as the author's own PASS, not as an independent review: One consequence before anyone "fixes" that line: I am not enqueueing this pull request. Both are body-level, so the tip |
This was referenced Oct 24, 2026
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
…cannot review (ThinkFlowLab#950) Owner: Dev Reviewer: Birfy Fixes ThinkFlowLab#947 Tip SHA: 4c11dc8 Written-by: Dev `Reviewer: Birfy` and not `Reviewer: Reviewer`, for one reason: `enqueue-guard.yml` runs the copy of this check on `main`, and that copy is the defect this pull request fixes — it refuses every Bot name. The reviewing workstation for this pull request is Reviewer; the line moves to `Reviewer: Reviewer` once this lands, and nothing else on the record changes. ## What changed - `.github/scripts/enqueue-guard.mjs` — `headerReasons` accepts a `Reviewer:` value that is either a `reviewers` login or a `writers` Bot name (leading `@` and case ignored, comma-separated as before); a word in neither is refused with two reasons that name the list it missed; and a line whose names are only `writers` Bot names is refused while `Birfy` — the identity every Bot writes as — is not in `reviewers`. One name from either list is enough for the line, and one bad name still refuses it. - `.github/scripts/enqueue-guard.test.mjs` — five new cases, plus the ThinkFlowLab#943 refusal case updated to the new pair of sentences. - `AGENTS.md`, `docs/autonomous-dev.md` — the two sentences that still said the value must be a login. Nothing else moves: the 20-line window, the draft and merged exemptions, `missing line: Owner:` / `missing line: Reviewer:`, the issue reference, the `agent-pr` label, the fork and mechanical-release exemptions and ThinkFlowLab#944's bot-account rule are as they were. ## Evidence Red on `ed60c5a` with only the new cases in the tree — 5 red, 43 green: ``` not ok 36 - ThinkFlowLab#943, reworded by ThinkFlowLab#947: a Reviewer: outside both vocabularies is refused and names the word not ok 38 - ThinkFlowLab#947: a Reviewer: naming a writers Bot name is accepted not ok 39 - ThinkFlowLab#947: either vocabulary is accepted, with an @ and in any case not ok 40 - ThinkFlowLab#947: a name in neither vocabulary is refused, and the refusal says which two lists it missed not ok 41 - ThinkFlowLab#947: a writers Bot name names nobody when the shared account is not a reviewer ``` Real bodies, judged as a live request (the `merged` exemption lifted), this branch left, `main` right: ``` ThinkFlowLab#942 Reviewer: Reviewer2 ok: true no header reason | ok: false Reviewer names "reviewer2", who is not in reviewers ThinkFlowLab#944 Reviewer: Reviewer ok: true no header reason | ok: false Reviewer names "reviewer", who is not in reviewers ThinkFlowLab#941 Reviewer: Birfy ok: true no header reason | ok: true no header reason ``` The boundary Chief asked for, run on the real policy with `Birfy` taken out of `reviewers`: ``` Reviewer: Reviewer2 ok: false Reviewer names "reviewer2", who is a writers Bot name but cannot record a Verdict here: every Bot writes as "Birfy", which is not in reviewers Reviewer: hsliuustc0106 ok: true no header reason ``` Mutations, each restored byte-identical afterwards: | mutation | red | | --- | --- | | drop the `writers` branch | 38, 39, 41, 42 | | `sharedLogin` always true | 41 | | drop the shared-login refusal | 41 | | back to the single old refusal sentence | 36, 40 | ## Ran - `node --test .github/scripts/enqueue-guard.test.mjs` → 48/48 pass - `node --test .github/scripts/review-gate.test.mjs` → 48/48 pass - `node --test .github/scripts/merge-gate.test.mjs` → 36/36 pass - `node --test .github/scripts/gate-check.test.mjs` → 16/16 pass - `node --test .github/scripts/*.test.mjs` → 389 tests, 383 pass, 4 fail, 2 skipped (the 4 are below) - `node .github/scripts/testing-layers.mjs --check` → 350 test files in 15 workflows, all classified - `node .github/scripts/changelog.mjs check` → 54 fragments well formed ## Not run - `npm run verify` and `oxlint`: this box has no `node_modules` and no dependency install, so the `core` suite, `scripts/lint-plugins.mjs` and the linter cannot run here. CI's `tests-ok` and `package` jobs run them at this tip. - The 4 failures in the full scripts run are all of `ds-bot-patches.test.mjs`, which needs `git`; there is none on this box, and `main`'s own tree fails the same four (384 tests, 378 pass, 4 fail). ## Risk T3 — `.github/**`, `AGENTS.md` and `docs/autonomous-dev.md` are on the T3 list, so this needs a PASS at this tip on top of green CI. The change is a vocabulary widening plus one refusal; it touches no workflow, no policy file and no merge path, and the check it edits is the one that reads the body. ## Scope One function in one gate, its tests, and two doc sentences. No policy file change, no second reviewers list, no change to who may PASS, no change to `review-gate` / `merge-gate` behaviour.
HugoXK
pushed a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 24, 2026
ThinkFlowLab#960) (ThinkFlowLab#969) Owner: Dev3 Reviewer: Reviewer Written-by: Dev3 Fixes ThinkFlowLab#960 Tip SHA: 0be0354 ## Reproduce On `2ab9e392`, `merge-gate` is the only gate a queue entry passes: it reads `hold` and the newest conclusion (`merge-gate.mjs:106-123`) and nothing else. The `main merge queue` ruleset requires three contexts — `Node 24 · DSH 0.2.0-rc.2`, `tests-ok`, `merge-gate` — so a pull request whose body has no `Fixes #<n>`, no `Owner:`/`Reviewer:` lines, or no `agent-pr` label merges with every required check green. `ThinkFlowLab#941` opened with `References ThinkFlowLab#874` alone and is the live example. The reason `enqueue-guard` is not one of the required contexts: `gate-check.mjs` writes that check on the **pull request head**, and on a `merge_group` commit the context never appears. Adding it to the ruleset today would put every entry on "Expected — waiting for status" — the `ThinkFlowLab#639` shape, measured in `ThinkFlowLab#960`. ## What changed - `.github/scripts/enqueue-rules.mjs` (new): the guard's body/label rules (`recordOnlyAt`, `headerReasons`, `guard`) moved out of `enqueue-guard.mjs`. Pure move — the block is byte-identical to `enqueue-guard.mjs`'s except `const short` gains `export`, so the two readers share one copy instead of two (ThinkFlowLab#812's kill condition). - `.github/scripts/merge-gate.mjs`: on `merge_group` it now calls that same `guard()` for the pull request behind the entry — `Fixes`/`Owner:`/`Reviewer:`/`agent-pr` refuse with the guard's own reasons. `fork` is read from `pull.head.repo.full_name`, so an external pull request keeps the ThinkFlowLab#711/ThinkFlowLab#713 exemption. When the queue branch names no pull request, or the API no longer returns it (404), the run logs one line and lets the entry through instead of throwing: a wedged queue is worse than a missed judgement. - `.github/scripts/gate-check.mjs` and `.github/workflows/enqueue-guard.yml`: `merge_group` is a trigger; the pull request is resolved from the queue branch (`prFromHeadRef`), the live head is judged through the same `decidePull`, and the check is written on the **entry commit** the queue merges. Every existing event and the printed command are unchanged. - Tests: 7 new in `merge-gate.test.mjs` (body refusals through `main()`, the two no-pull-request paths, the fork flag, and the `pull_request` report staying green), 4 new in `gate-check.test.mjs` (entry-commit target, refusal, PATCH-not-second-run, workflow wiring). - `AGENTS.md`, `docs/autonomous-dev.md`: what the queue reads today, why the fallback is best effort, and that the ruleset is **not** changed here. ## What I ran - `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` → **179/179** (59/43/48/20/9; base has 168: 59/36/48/16/9). - `cd .github/scripts && node --test "*.test.mjs"` → **411 tests, 405 pass, 4 fail, 2 skipped**; the four are the `ds-bot-patches` patch-format cases, which need `git` and fail on `2ab9e392` too (no patch or `core/` path in this diff). - The moved block: `sed -n 31,190p` of the base `enqueue-guard.mjs` vs the new `enqueue-rules.mjs` body differs only in the `export` keyword on `short`. ## Red without the source change On pristine `2ab9e392` with only the new tests added: **63 tests, 53 pass, 10 fail** — 6 of the 7 new `merge-gate` tests (the seventh is the `pull_request` report control and must stay green) and all 4 new `gate-check` tests. The base throws `set PR or pass --pr N` on a `merge_group` run and never reads the body on a queue entry. Mutations, each restored afterwards: - disable the `guard()` call in `merge-gate.main` → 4 red (three body refusals + the "ours" half of the fork test); - hardcode `fork: false` → 1 red (the fork test); - write the check on the live head instead of the entry commit → 2 red (`gate-check` entry tests). ## Not run `npm run verify` (no `core/` changes and no core harness in this tree), `ds-bot-patches` regeneration (no generated path touched), smoke test. No `changelog.d` fragment: internal gate wiring, as `ThinkFlowLab#947`/`ThinkFlowLab#950`/`ThinkFlowLab#953`/`ThinkFlowLab#962`. **No ruleset change**: `main merge queue` still requires the same three contexts; making `enqueue-guard` a fourth is the separate decision ThinkFlowLab#960 describes, and it is gated on this production path being proven. ## Risk The fallback is deliberately best effort: an unparsable queue branch or a pull request the API drops is a missed judgement, logged in the job summary, never a wedge. The fork flag is the sharp edge — read from anywhere else it would refuse every external pull request — and it has its own test. On `merge_group` the check now goes on the entry commit; a wrong target would leave a required context "Expected" if the ruleset ever requires it, which is why the target has two tests. `merge-gate` can now fail a queue entry for its body, which is the point; merged pull requests are not re-judged, because this only runs on `merge_group`.
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
Owner: Dev
Reviewer: Reviewer2
References #874
Fixes #946
Step 2 only. This does not close the RFC.
What changed
An offline experience library, keyed by repository and tech stack, kept separate from root
LESSONS.md.core/dev/experience/repos/<owner>/<name>/andcore/dev/experience/stacks/<techStack>/.stale-baseandone-conclusion, plus shared stacknode-host/generated-gates.lookupExperienceandrt.harnessExperienceread one scope. A repo lookup opens that repo's directory only. A tech-stack lookup returns entries withrepo: null. A call with neither key returns an empty list. There is no model call.node core/dev/experience/check.mjschecks the tree.addwrites one new file and refuses to overwrite. It does not edit harness configs.appliedstays false.harnessProstill names CI,core/dev/eval, andrsi-baseline.The read API lives in
core/src/host/experience.jsso the host can call it. The JSON library stays incore/dev/experience/, which the npm package does not ship. A packaged install's default read returns no entries.Out of scope
Option D (an online self-modifying harness). Injecting lessons into the prompt. Changing the gate or enqueue-guard. Replacing CI, eval, or rsi-baseline. Merging this pull request.
How to test
Verification at the tip
Tip SHA:
cd7b314d752f2d3800cf846ed0aba3532d1c90e0Ran:
experience.jsexisted:node --import ./test/harness-resolve.mjs --test test/experience.test.jsfailed withERR_MODULE_NOT_FOUND(0 pass / 1 fail).TZ=UTC npm testincore/: 1380 pass, 0 fail.npm run lintincore/: pass. The new files added no oxlint warnings.node core/scripts/check-arch-docs.mjsat this tip: OK (18 changed files).node scripts/ds-bot-patches.mjs check: pass.node core/scripts/build-client.mjs --check: pass.node scripts/lint-plugins.mjs: pass.node --test .github/scripts/*.test.mjs: 370 pass.npm run test:interop: 37 pass.npm run test:live-e2e: 16 pass. Interop and live-e2e lint passed in the same run.Not run: the smoke bench (
dev/bench.sh/dev/smoke.mjs). The new hook is not on the turn path; the core suite loadsharness.js. No changelog fragment: this is maintainer and agent tooling, not a user-visible product change.