From 599034899eadea4efb029d8d6c7f4754b0ec8119 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 07:35:57 -0400 Subject: [PATCH 1/2] feat(agents): build+test gate before pre-PR review; tighter current-context lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related changes to the triage/refresh pipeline: 1. triage-prompt.md — adds a "Pre-PR build + test gate" section before the expert review. Runs `npm run precommit` (or equivalent for doc-only diffs), capped at 2 build→fix iterations. Rationale: expert review is expensive; don't spawn subagents on broken code. And a red PR at push time makes the planned CI auto-fix loop indistinguishable from drift-after-merge. 2. context-refresh-prompt.md — adds an explicit rule: bullet labels must be factual nouns ("**Compliance storyboard remediation**"), not editorial framing ("**Compliance storyboard gaps**"). The lint now enforces this at the error level. 3. validate-agent-context.mjs — adds LABEL_BAN regex that hard-fails when gap/risk/concern/narrative/stakeholder appears inside a bold span. Tier and editorial are deliberately omitted (AdCP has legitimate product-level uses: "Tier-2 Production Verified", "editorial workflow"). Single-line regex — won't span paragraphs. Sibling repos (adcp-client, adcp-client-python, adcp-go) get the build+test-gate mirror in a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) --- .agents/routines/context-refresh-prompt.md | 7 ++++++ .agents/routines/triage-prompt.md | 28 +++++++++++++++++++++- .changeset/triage-build-test-gate.md | 4 ++++ .github/scripts/validate-agent-context.mjs | 22 ++++++++++++++++- 4 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 .changeset/triage-build-test-gate.md diff --git a/.agents/routines/context-refresh-prompt.md b/.agents/routines/context-refresh-prompt.md index c06ac4ca2f..139af1264c 100644 --- a/.agents/routines/context-refresh-prompt.md +++ b/.agents/routines/context-refresh-prompt.md @@ -70,6 +70,13 @@ routine's context. - Link every entry (`#1234`, `PR adcp-client#456`) - Status values: `active`, `blocked`, `review`, `shipped`, `deferred` - Drop stale entries — staleness is a feature +- **Bullet labels must be factual nouns.** Not "**X gaps**", + "**X risks**", "**X concerns**", "**Tier-N X**", or + "**Stakeholder X**". Say what the work *is*, not how it's + framed. Example: "**Compliance storyboard remediation**", not + "**Compliance storyboard gaps**". The lint flags these, and + any gap/risk/concern/tier framing belongs in + `internal-context.md` instead. ## Open a PR diff --git a/.agents/routines/triage-prompt.md b/.agents/routines/triage-prompt.md index 83410a259b..57a17a8d5b 100644 --- a/.agents/routines/triage-prompt.md +++ b/.agents/routines/triage-prompt.md @@ -424,9 +424,35 @@ A single cohesive PR of 200 non-breaking lines is easier to review than three PRs of 60 lines with dependencies and cross-links. The bot's job is to reduce maintainer clicks, not multiply them. +## Pre-PR build + test gate — mandatory before expert review + +The expert review is expensive; don't run it on broken code. Before +spawning experts, make sure the diff actually compiles and the +unit tests pass. + +1. Run the repo's build + fast test tier: + - `npm run precommit` — prefer this (bundled build + typecheck + + unit tests); falls back to `npm run build && npm test` if not + defined + - If the diff touches only `docs/**` or `static/**`, skip build + and run the relevant doc check instead (e.g., `npm run + docs:check` or `mintlify broken-links`) +2. **If build or tests fail:** read the errors, fix the code, + re-run. Cap at **2 build→fix iterations.** If still failing, + abandon the PR and Flag for human review with the build log + in the comment. +3. Do **not** skip tests locally because "CI will run them." The + point of this gate is to not ship known-broken code even as a + draft, because (a) review noise, (b) a human reviewer may + admin-merge a draft that looks fine, (c) a green CI on push + is the baseline for the auto-fix loop — a red PR at push time + is indistinguishable from drift after the fact. +4. Only once build + tests pass on the final diff: proceed to + pre-PR expert review. + ## Pre-PR expert review — mandatory before `gh pr create` -After the branch is pushed but **before** opening the PR, run a +After build + tests are green but **before** opening the PR, run a second expert pass on the actual diff. The Step 4 synthesis reviewed the plan; this step reviews the code. They catch different things — protocol drift, broken tests, overlong files, diff --git a/.changeset/triage-build-test-gate.md b/.changeset/triage-build-test-gate.md new file mode 100644 index 0000000000..7ce30b081b --- /dev/null +++ b/.changeset/triage-build-test-gate.md @@ -0,0 +1,4 @@ +--- +--- + +Triage routine now runs a mandatory pre-PR build + test gate (npm run precommit or equivalent) before expert review, with 2 build→fix iterations. Also elevates bullet-label boundary framing ("**X gaps**") from CI warning to hard error in the current-context lint, and adds an explicit rule to the context-refresh prompt. diff --git a/.github/scripts/validate-agent-context.mjs b/.github/scripts/validate-agent-context.mjs index d46e01d73c..28009fe7dd 100755 --- a/.github/scripts/validate-agent-context.mjs +++ b/.github/scripts/validate-agent-context.mjs @@ -82,7 +82,27 @@ if (/[\x00-\x08\x0B\x0C\x0E-\x1F]/.test(content)) { errors.push('Control characters present. Strip them.'); } -// -- Boundary checks (warnings) ---------------------------------------- +// -- Boundary checks --------------------------------------------------- + +// Hard fail: boundary framing inside a **bold** span — bullet labels +// and section emphasis are the load-bearing signal. "Compliance +// storyboard gaps" as a bullet label reads like internal framing; +// "filter-behaviour gap [#2902]" mid-sentence is describing a linked +// issue and is fine. +// "tier-N" and "editorial" are deliberately omitted — AdCP has +// legitimate product concepts using both (e.g. "Tier-2 Production +// Verified", "Member editorial workflow"). They stay as +// BOUNDARY_PATTERNS warnings for subtler cases. +const LABEL_BAN = /\*\*[^*\n]*\b(?:gap|risk|concern|narrative|stakeholder)s?\b[^*\n]*\*\*/i; +const labelMatch = content.match(LABEL_BAN); +if (labelMatch) { + errors.push( + `Boundary framing inside a bold span: "${labelMatch[0]}". Bullet ` + + `labels and section emphasis should be factual nouns. Reword ` + + `(e.g. "gaps" → "remediation") or move the entry to ` + + `\`.agents/internal-context.md\`.` + ); +} const BOUNDARY_PATTERNS = [ [/\btier[- ][0-9]\b/i, 'priority tier ("tier-1")'], From 16dc9c7703bd795e8673c62830431030c68abb73 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 07:39:44 -0400 Subject: [PATCH 2/2] test(rules-loader): drop stale AdCP Agent Types assertion #2921 removed the hardcoded AdCP Agent Types section from server/src/addie/rules/knowledge.md (Addie now defers to search_docs for taxonomy questions). This assertion was pre-existing broken on main after #2921 merged; not specific to any PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- server/tests/unit/rules-loader.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server/tests/unit/rules-loader.test.ts b/server/tests/unit/rules-loader.test.ts index 8e7986cac3..e097d7435c 100644 --- a/server/tests/unit/rules-loader.test.ts +++ b/server/tests/unit/rules-loader.test.ts @@ -38,7 +38,6 @@ describe('Rules Loader', () => { expect(rules).toContain('## Partner Directory'); // Knowledge - expect(rules).toContain('## AdCP Agent Types'); expect(rules).toContain('## Prebid Expertise'); expect(rules).toContain('## Trusted Match Protocol (TMP)');