Skip to content

Commit 2291dca

Browse files
Restore Greybeard review checklist in Corbits idiom (#944)
* Restore Greybeard review checklist in Corbits idiom * Assert Greybeard checklist order and verdict-scoped triad in tests
1 parent 9ff51db commit 2291dca

2 files changed

Lines changed: 54 additions & 6 deletions

File tree

src/agent/directors/greybeard/package.test.ts

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,52 @@ describe("greybeardPackage", () => {
2222
expect(p).not.toMatch(/architecture director/i);
2323
});
2424

25-
test("systemPrompt teaches judgment for architecture approach", () => {
25+
test("systemPrompt frames value as analysis via Corbits read tools", () => {
2626
const p = greybeardPackage.systemPrompt;
27-
expect(p).toContain("Judge the approach");
27+
expect(p).toMatch(/value is analysis/i);
28+
expect(p).toContain("read_file");
29+
expect(p).toContain("grep");
30+
expect(p).toContain("ask_director");
31+
});
32+
33+
test("systemPrompt carries an ordered review checklist", () => {
34+
const p = greybeardPackage.systemPrompt;
35+
expect(p).toMatch(/Review checklist/);
36+
expect(p).toMatch(/architectural claim/);
2837
expect(p).toMatch(/constraint ownership|owns constraints/i);
29-
expect(p).toMatch(/hold \/ revise \/ block|verdict/i);
38+
expect(p).toMatch(/anti-patterns/);
39+
expect(p).toMatch(/Rank risks/);
40+
const checklistIdx = p.search(/Review checklist/);
41+
expect(checklistIdx).toBeGreaterThan(-1);
42+
const checklist = p.slice(checklistIdx);
43+
const claimIdx = checklist.search(/architectural claim/);
44+
const ownershipIdx = checklist.search(/constraint ownership|owns constraints/i);
45+
const holesIdx = checklist.search(/anti-patterns/);
46+
const risksIdx = checklist.search(/Rank risks/);
47+
const verdictIdx = checklist.search(/hold \/ revise \/ block/);
48+
expect(claimIdx).toBeGreaterThan(-1);
49+
expect(ownershipIdx).toBeGreaterThan(claimIdx);
50+
expect(holesIdx).toBeGreaterThan(ownershipIdx);
51+
expect(risksIdx).toBeGreaterThan(holesIdx);
52+
expect(verdictIdx).toBeGreaterThan(risksIdx);
53+
});
54+
55+
test("systemPrompt ends the checklist with the hold/revise/block verdict triad", () => {
56+
const p = greybeardPackage.systemPrompt;
57+
expect(p).toMatch(/hold \/ revise \/ block/);
3058
expect(p).toMatch(/backward-compatibility|backward compatibility/i);
59+
const risksIdx = p.search(/Rank risks/);
60+
const triadIdx = p.search(/hold \/ revise \/ block/);
61+
expect(risksIdx).toBeGreaterThan(-1);
62+
expect(triadIdx).toBeGreaterThan(risksIdx);
63+
});
64+
65+
test("systemPrompt has no self-spawn language", () => {
66+
const p = greybeardPackage.systemPrompt;
67+
expect(p).not.toMatch(/spawn.*greybeard/i);
68+
expect(p).not.toContain('agent="greybeard"');
69+
expect(p).not.toMatch(/spawn yourself/i);
70+
expect(p).not.toMatch(/spawn a (greybeard|reviewer)/i);
3171
});
3272

3373
test("systemPrompt allows limited spawn without fake caps or scheduler language", () => {

src/agent/directors/greybeard/package.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import { ORCHESTRATOR_TOOLS } from "../tool-sets.js";
33

44
/**
55
* Greybeard nested orchestrator (CL-7019).
6-
* Architecture judgment with limited spawn — never ships product code.
6+
* Review checklist ported from the GaaS greybeard original (CL-7662) — the
7+
* GaaS source was unavailable locally, so this is a Corbits-idiom restoration
8+
* rather than a 1:1 copy. Self-read deviation: the GaaS delegate-for-review
9+
* shape becomes read_file/grep/ask_director first, spawn only on a concrete
10+
* unknown. Architecture judgment with limited spawn — never ships product code.
711
*/
812
export const greybeardPackage: DirectorPackage = {
913
id: "greybeard",
@@ -26,12 +30,16 @@ You are Greybeard — not a second Skywalker, not Critic (code defects with evid
2630
2731
Follow style and philosophy conventions (baked into this prompt) when reviewing plans or approaches — skills are active constraints, not background docs.
2832
29-
Judge the approach:
33+
Your value is analysis, not delegation: reach the judgment yourself with
34+
targeted reads (read_file, grep) and pointed questions (ask_director)
35+
before considering a spawn.
36+
37+
Review checklist — work the list in order:
3038
1. Name the architectural claim under review (boundary, ownership, invariant, or BC surface).
3139
2. Decide whether the proposed approach owns constraints at the right layer — or only chases symptoms.
3240
3. Call out holes, anti-patterns, missing invariants, product/architecture/implementation misalignment, and duplication that should be refactor or API expansion instead.
3341
4. Rank risks for long-term maintainability and backward compatibility.
34-
5. Report a clear verdict: hold / revise / block — with the why, not a checklist theater.
42+
5. Report a clear verdict: hold / revise / block — with the why, not checklist theater.
3543
3644
Spawn only when a concrete unknown blocks that judgment. Package spawn rules allow intern (mechanical shell), explorer (map/read), and critic (code evidence). When spawning critic, pass non-empty success_criteria (runtime fail-closes without it). intern and explorer remain optional. Prefer doing the review yourself with mounted read/search tools. Do not invent numeric spawn caps or act as a scheduler — width follows the unknown, not a soft ladder. Spawn then idle; reports arrive as mailbox mail — do not poll.
3745

0 commit comments

Comments
 (0)