Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 99 additions & 44 deletions packages/agents/content/__tests__/guidance-hook-reach.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,46 +6,88 @@ import { describe, expect, it } from 'vitest';
import { readInjectedSkills } from '../../src/lib/dependency-frontmatter.ts';
import { expandIncludes } from '../../src/lib/directive-expander.ts';
import { parseFrontmatter } from '../../src/lib/frontmatter-merger.ts';
import type { GuidanceHookFills } from '../../src/lib/guidance-hooks.ts';
import type { GuidanceHookFill, GuidanceHookFills } from '../../src/lib/guidance-hooks.ts';
import { assertFilledAnchorsResolve, fillGuidanceHooks, listGuidanceHooks } from '../../src/lib/guidance-hooks.ts';
import { parseRulebookFile } from '../../src/lib/rulebook-schema.ts';

// A guidance hook reaches an agent two ways, and both are checked here: a body declares the directive itself, or a
// subagent preloads a skill that declares it. Each route is one line an edit can drop with no other test failing.
//
// Every hook is a row of one table rather than a file of its own, so a hook added with no row is visible as an absence
// here instead of as a suite nobody wrote.
const CONTENT_ROOT = new URL('../', import.meta.url).pathname;
const RULEBOOKS_ROOT = path.join(CONTENT_ROOT, 'guidance', 'rulebooks');
const SUBAGENTS_ROOT = path.join(CONTENT_ROOT, 'subagents');

const HOOK = 'implementation-preferences';
/** A rulebook a declaration binds to a hook, with a phrase that would not survive the rulebook being gutted. */
interface BoundRulebook {
readonly slug: string;
readonly rule: string;
}

/** A body declaring the hook: the slug naming it in a test title, and its path under the content root. */
/** A body declaring a hook: the slug naming it in a test title, and its path under the content root. */
interface DeclaringBody {
readonly label: string;
readonly relativePath: string;
}

/** A rulebook a declaration binds to the hook, with a phrase that would not survive the rulebook being gutted. */
interface BoundRulebook {
readonly slug: string;
readonly rule: string;
/** One hook, the bodies that declare it, the rulebooks bound to it, and the body its splice is proven against. */
interface HookGuard {
readonly hook: string;
readonly role: string;
readonly declaringBodies: ReadonlyArray<DeclaringBody>;
readonly boundRulebooks: ReadonlyArray<BoundRulebook>;
readonly spliceProbe: SpliceProbe;
}

/** The body one hook's splice is asserted against, and injected text a fill must leave standing beside its own. */
interface SpliceProbe {
readonly body: DeclaringBody;
readonly coexisting: ReadonlyArray<string>;
}

// Listed explicitly rather than discovered from the directives: the failure guarded against is a body dropping off,
// and a discovered list would move with the bug.
const DECLARING_BODIES: ReadonlyArray<DeclaringBody> = [
{ label: 'implement-plan', relativePath: 'skills/implement-plan/SKILL.md' },
{ label: 'orchestrated-coder', relativePath: 'subagents/orchestrated-coder.md' },
{ label: 'respond-to-review', relativePath: 'skills/respond-to-review/SKILL.md' },
{ label: 'review-branch', relativePath: 'skills/review-branch/SKILL.md' },
{ label: 'review-criteria', relativePath: 'skills/review-criteria/SKILL.md' },
];

const BOUND_RULEBOOKS: ReadonlyArray<BoundRulebook> = [
{ slug: 'williamthorsen-code-layout-preferences', rule: 'A `__tests__/` directory sits beside the code it covers' },
{ slug: 'williamthorsen-typescript-preferences', rule: 'Never use a type assertion' },
const HOOK_GUARDS: ReadonlyArray<HookGuard> = [
{
hook: 'implementation-preferences',
role: 'writes or judges code',
declaringBodies: [
{ label: 'implement-plan', relativePath: 'skills/implement-plan/SKILL.md' },
{ label: 'orchestrated-coder', relativePath: 'subagents/orchestrated-coder.md' },
{ label: 'respond-to-review', relativePath: 'skills/respond-to-review/SKILL.md' },
{ label: 'review-branch', relativePath: 'skills/review-branch/SKILL.md' },
{ label: 'review-criteria', relativePath: 'skills/review-criteria/SKILL.md' },
],
boundRulebooks: [
{
slug: 'williamthorsen-code-layout-preferences',
rule: 'A `__tests__/` directory sits beside the code it covers',
},
{ slug: 'williamthorsen-typescript-preferences', rule: 'Never use a type assertion' },
],
spliceProbe: {
body: { label: 'implement-plan', relativePath: 'skills/implement-plan/SKILL.md' },
coexisting: ['## Comment discipline'],
},
},
{
hook: 'ticketing-preferences',
role: 'decides on or creates tickets',
declaringBodies: [
{ label: 'create-ticket', relativePath: 'skills/create-ticket/SKILL.md' },
{ label: 'design-and-plan', relativePath: 'skills/design-and-plan/SKILL.md' },
{ label: 'respond-to-review', relativePath: 'skills/respond-to-review/SKILL.md' },
],
boundRulebooks: [{ slug: 'williamthorsen-ticketing-preferences', rule: 'give each pull request its own ticket' }],
spliceProbe: {
body: { label: 'create-ticket', relativePath: 'skills/create-ticket/SKILL.md' },
coexisting: ['{Clear statement of what needs to be solved and why}'],
},
},
];

/** The skill every reviewer subagent preloads, and so the one that carries the hook to all of them. */
/** The skill every reviewer subagent preloads, and so the one that carries the hooks it declares to all of them. */
const REVIEWER_CARRIER = 'review-criteria';

const REVIEWER_SUBAGENTS: ReadonlyArray<string> = [
Expand All @@ -56,58 +98,71 @@ const REVIEWER_SUBAGENTS: ReadonlyArray<string> = [
'orchestrated-reviewer',
];

describe('guidance-hook reach', () => {
it.each(DECLARING_BODIES)('$label declares the hook', async ({ label, relativePath }) => {
describe.each(HOOK_GUARDS)('$hook reach', ({ boundRulebooks, declaringBodies, hook, role, spliceProbe }) => {
it.each(declaringBodies)('$label declares the hook', async ({ label, relativePath }) => {
const declared = listGuidanceHooks(await expandBody(relativePath), label).map(({ name }) => name);

const message = `${label} writes or judges code but declares no ${HOOK} hook, so a binding cannot reach it`;
expect(declared, message).toContain(HOOK);
const message = `${label} ${role} but declares no ${hook} hook, so a binding cannot reach it`;
expect(declared, message).toContain(hook);
});

it.each(BOUND_RULEBOOKS)('$slug declares hook delivery', async ({ slug }) => {
it.each(boundRulebooks)('$slug declares hook delivery', async ({ slug }) => {
const { rulebook } = parseRulebookFile(await readFile(path.join(RULEBOOKS_ROOT, `${slug}.md`), 'utf8'), slug);

const message = `${slug} is bound to ${HOOK} but its delivery does not name the route, so sync warns about it`;
const message = `${slug} is bound to ${hook} but its delivery does not name the route, so sync warns about it`;
expect(rulebook.delivery, message).toContain('hook');
});

it.each(REVIEWER_SUBAGENTS)('%s preloads the skill declaring the hook', async (slug) => {
const content = await readFile(path.join(SUBAGENTS_ROOT, `${slug}.md`), 'utf8');
const injected = readInjectedSkills(content, `${slug}.md`);

const message = `${slug} judges code but preloads no ${REVIEWER_CARRIER}; injected: [${injected.join(', ')}]`;
expect(injected, message).toContain(REVIEWER_CARRIER);
});

it('splices every bound rulebook into a declaring body', async () => {
const label = 'implement-plan';
const filled = fillGuidanceHooks(await expandBody(`skills/${label}/SKILL.md`), await buildFills(), label);
const { body, coexisting } = spliceProbe;
const filled = fillGuidanceHooks(await expandBody(body.relativePath), await buildFills(), body.label);

for (const { rule } of BOUND_RULEBOOKS) {
for (const { rule } of boundRulebooks) {
expect(filled.content).toContain(rule);
}
expect(filled.content).toContain('## Comment discipline');
for (const text of coexisting) {
expect(filled.content).toContain(text);
}
});

it.each(DECLARING_BODIES)('$label resolves its anchors once filled', async ({ label, relativePath }) => {
it.each(declaringBodies)('$label resolves its anchors once filled', async ({ label, relativePath }) => {
const filled = fillGuidanceHooks(await expandBody(relativePath), await buildFills(), label);

expect(() => assertFilledAnchorsResolve(filled, label)).not.toThrow();
});
});

// The reviewer subagents reach a hook through a preloaded skill rather than a directive of their own, a route only
// `implementation-preferences` takes. Kept beside the table rather than in it, so no other hook carries an empty field
// for a route it does not use.
describe('reviewer-subagent carrier', () => {
it.each(REVIEWER_SUBAGENTS)('%s preloads the skill declaring the hook', async (slug) => {
const content = await readFile(path.join(SUBAGENTS_ROOT, `${slug}.md`), 'utf8');
const injected = readInjectedSkills(content, `${slug}.md`);

const message = `${slug} judges code but preloads no ${REVIEWER_CARRIER}; injected: [${injected.join(', ')}]`;
expect(injected, message).toContain(REVIEWER_CARRIER);
});
});

// region | Helpers

/**
* Builds the fills a declaration produces, keyed by hook. Bound bodies arrive unrendered: link rewriting and
* invocation-token resolution belong to `sync` and are covered there, and what these assertions cover is the splice
* into the real consumer bodies.
* Builds the fills a declaration produces, keyed by hook, spanning every guard rather than one hook at a time: a body
* can declare more than one, and an anchor collision only shows up once they fill together the way `sync` fills them.
* Bound bodies arrive unrendered: link rewriting and invocation-token resolution belong to `sync` and are covered
* there, and what these assertions cover is the splice into the real consumer bodies.
*/
async function buildFills(): Promise<GuidanceHookFills> {
const bound = await Promise.all(
BOUND_RULEBOOKS.map(async ({ slug }) => ({ slug, body: await readRulebookBody(slug) })),
const entries = await Promise.all(
HOOK_GUARDS.map(async ({ boundRulebooks, hook }): Promise<[string, ReadonlyArray<GuidanceHookFill>]> => {
const bound = await Promise.all(
boundRulebooks.map(async ({ slug }) => ({ slug, body: await readRulebookBody(slug) })),
);
return [hook, bound];
}),
);
return new Map([[HOOK, bound]]);
return new Map(entries);
}

/** Returns a skill or subagent body with its includes expanded — the body the deploy pipeline goes on to fill. */
Expand Down
3 changes: 2 additions & 1 deletion packages/agents/content/collections/williamthorsen.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ members:
- williamthorsen-code-layout-preferences
- williamthorsen-collaboration-preferences
- williamthorsen-comment-preferences
- williamthorsen-ticketing-preferences
- williamthorsen-tooling-preferences
- williamthorsen-typescript-preferences
- williamthorsen-workflow-preferences
Expand All @@ -16,7 +17,7 @@ members:

# William Thorsen

The personal collection. Membership claims an artifact was examined and found deliberately fitted to one author rather than generally applicable. What disqualifies each member from `recommended` is the reason it belongs here: the seven rulebooks state one author's preferences as rules another team would answer differently, and `people-report` serves a domain one author works in rather than one every project shares.
The personal collection. Membership claims an artifact was examined and found deliberately fitted to one author rather than generally applicable. What disqualifies each member from `recommended` is the reason it belongs here: the eight rulebooks state one author's preferences as rules another team would answer differently, and `people-report` serves a domain one author works in rather than one every project shares.

Its closure reaches `capture-feedback` and `capture-event` in `recommended` and nothing else, which is what makes declaring it safe alongside the public collection.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
slug: williamthorsen-ticketing-preferences
description: William Thorsen's preferences for how work is split across tickets and how the relationships between them are recorded.
delivery: hook
version: 1
---

# William Thorsen's ticketing preferences

## Splitting work across tickets

When work deserves more than one pull request, give each pull request its own ticket. When the split yields more than two tickets, the originating ticket becomes an umbrella rather than carrying work of its own.

Record the relationships natively where the tracker supports them -- blockers, and parent to child -- rather than as prose in a body.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
slug: williamthorsen-workflow-preferences
description: William Thorsen's personal preferences for how work moves -- scope, branches and worktrees, guidance capture, and how work is split across tickets.
description: William Thorsen's personal preferences for how work moves -- scope, branches and worktrees, and guidance capture.
delivery: ambient
version: 1
version: 2
---

# William Thorsen's workflow preferences
Expand All @@ -15,12 +15,6 @@ version: 1
- When feedback should change how the agent behaves and generalizes beyond the current task, capture it via {skill:capture-feedback}, which routes it to guidance refinement that propagates to every project and machine. Do not record generalizable guidance as a per-project memory.
- Memories are scoped to a single project on a single machine, so using them for generalizable guidance fragments behavior across contexts. Reserve them for genuinely local, non-propagating facts (a project-specific deadline or quirk).

## Splitting work across tickets

When work deserves more than one pull request, give each pull request its own ticket. When the split yields more than two tickets, the originating ticket becomes an umbrella rather than carrying work of its own.

Record the relationships natively where the tracker supports them -- blockers, and parent to child -- rather than as prose in a body.

## Branch and worktree management

Never create a branch or worktree without explicit authorization. Never switch a worktree to another branch; one ticket's work happens entirely in one worktree, and work on another ticket starts a new session in that ticket's own worktree.
Expand Down
2 changes: 2 additions & 0 deletions packages/agents/content/skills/create-ticket/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Create a ticket on the appropriate platform. The remote platform (e.g., GitHub)

Each takes ticket references in the project's own form (`#1163`, `MAC-42`), comma-separated where more than one applies. All three are optional, and each overrides the inference in step 4 for its own relationship.

<!-- guidance-hook: ticketing-preferences -->

## Process

### 1. Resolve project metadata
Expand Down
2 changes: 2 additions & 0 deletions packages/agents/content/skills/design-and-plan/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Turn a task into a well-defined ticket and actionable implementation plan throug
Do NOT generate the implementation plan until the design has been agreed upon and the ticket has been approved. This applies regardless of perceived simplicity.
</HARD-GATE>

<!-- guidance-hook: ticketing-preferences -->

## Process

**Narrate every ask.** Throughout the phases below, before presenting any ask to the user — a clarifying question, a design or ticket or plan approval, a drift-verdict decision, the closing next-steps menu — emit `input.requested` with a short label of the ask (e.g. payload `{"prompt":"tier selection"}`) per [Lifecycle events](#lifecycle-events), so a watching surface shows which question this session is waiting on. Compose the label fresh for each ask and keep it to a few words. Emit before presenting, in the same turn — never at the end of the turn.
Expand Down
2 changes: 2 additions & 0 deletions packages/agents/content/skills/respond-to-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ If no path is provided, find the most recent `reviewer_review` in the active run

If no review artifact is found, stop and report the error.

<!-- guidance-hook: ticketing-preferences -->

## Evaluation protocol

For each finding, apply technical rigor:
Expand Down
Loading