From f5237ea3c75759788035d9acaa47a401d2085204 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 12 Jul 2026 10:22:22 -0700 Subject: [PATCH 1/5] agents|tests: Add a link-resolution guard for installable content A relative Markdown link whose target has moved or been deleted now fails the build instead of shipping to installed skills and subagents, where it would resolve to nothing. --- .../__tests__/content-link-resolution.test.ts | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 packages/agents/src/__tests__/content-link-resolution.test.ts diff --git a/packages/agents/src/__tests__/content-link-resolution.test.ts b/packages/agents/src/__tests__/content-link-resolution.test.ts new file mode 100644 index 00000000..8b12a413 --- /dev/null +++ b/packages/agents/src/__tests__/content-link-resolution.test.ts @@ -0,0 +1,99 @@ +import { existsSync } from 'node:fs'; +import { readdir } from 'node:fs/promises'; +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { expandIncludes } from '../lib/directive-expander.ts'; + +// A relative Markdown link in installable content is rewritten at install time by `rewriteMarkdownPaths`, which +// resolves it against the *host* file's directory — the skill or subagent the link renders into, not the partial it +// may have been authored in. A link whose target has moved or been deleted still installs cleanly, leaving the agent +// to follow a link to nothing. Resolving every link the way the installer does turns that into a build failure. +// +// Host roots only. A `_partials/` file is never installed standalone, and its links are authored against the host that +// inlines it — checking one in isolation would misresolve every `../` it carries. Include expansion below reaches them +// through each host, which is the only context where they mean anything. `guidance/` is copied verbatim with no link +// rewriting, so it is out of scope. +const HOST_ROOTS: ReadonlyArray = ['skills', 'subagents']; + +const CONTENT_ROOT = new URL('../../content/', import.meta.url).pathname; + +const MARKDOWN_LINK_REGEX = /\[[^\]]*\]\(([^)]+)\)/g; + +interface Violation { + readonly file: string; + readonly target: string; +} + +/** Recursively collects installable host `.md` files, skipping `_partials/` at any depth and dotfiles. */ +async function collectHostFiles(dir: string, out: Array): Promise { + for (const entry of await readdir(dir, { withFileTypes: true })) { + if (entry.name === '_partials' || entry.name.startsWith('.')) { + continue; + } + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + await collectHostFiles(full, out); + } else if (entry.isFile() && entry.name.endsWith('.md')) { + out.push(full); + } + } +} + +async function findViolations(): Promise> { + const hostFiles: Array = []; + for (const root of HOST_ROOTS) { + await collectHostFiles(path.join(CONTENT_ROOT, root), hostFiles); + } + hostFiles.sort(); + + const violations: Array = []; + for (const hostFile of hostFiles) { + const expanded = await expandIncludes(hostFile, CONTENT_ROOT); + for (const match of expanded.matchAll(MARKDOWN_LINK_REGEX)) { + const target = match[1]; + if (target === undefined || !isRelativeTarget(target)) { + continue; + } + const targetPath = target.split('#')[0]; + if (targetPath === undefined || targetPath === '') { + continue; + } + if (!existsSync(path.resolve(path.dirname(hostFile), targetPath))) { + violations.push({ file: path.relative(CONTENT_ROOT, hostFile), target }); + } + } + } + return violations; +} + +function formatViolations(violations: ReadonlyArray): string { + if (violations.length === 0) { + return ''; + } + const header = + `Found ${violations.length} relative Markdown link(s) whose target does not exist. Each is resolved against the ` + + `host file's directory, matching how \`rewriteMarkdownPaths\` resolves it at install time. A link authored in a ` + + `partial is reported against every host that inlines it, so fix the partial rather than the host.`; + const lines = violations.map((v) => ` ${v.file}: ${v.target}`); + return [header, ...lines].join('\n'); +} + +/** Reports whether a link target is a relative path — the only form the install pipeline rewrites. */ +function isRelativeTarget(target: string): boolean { + return !( + /^https?:\/\//.test(target) || + target.startsWith('/') || + target.startsWith('~') || + target.startsWith('#') || + target.startsWith('{') + ); +} + +describe('installable-content link resolution', () => { + it('every relative Markdown link in an installable host resolves to a real file', async () => { + const violations = await findViolations(); + expect(violations, formatViolations(violations)).toEqual([]); + }); +}); From 723065f6a61b63a3b242c52c2d7be46a47f80587 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 12 Jul 2026 10:22:34 -0700 Subject: [PATCH 2/5] agents|fix: Inline output-shaping specs so skills cannot improvise them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every skill that renders a next-steps menu or an option-style question now carries those blocks in its own text, so producing them no longer depends on the agent choosing to follow a link first. That read was optional at the moment the output was generated, and it was being skipped: menus came out with their numeric labels dropped, or with a recommendation that did not match the option carrying the strongest marker. The wider doctrine behind the format — when a confirmation prompt fits better than a numbered list, how to rank options against one another — remains a linked reference for the skills that consult it. --- .../skills/_data/recommendation-gradient.md | 50 +------- .../next-steps-after-plan.md | 18 ++- .../next-steps-after-review.md | 54 ++++---- .../content/skills/_partials/option-format.md | 36 ++++++ .../content/skills/collaborate/SKILL.md | 4 +- .../content/skills/design-and-plan/SKILL.md | 10 +- .../agents/content/skills/merge-pr/SKILL.md | 4 +- .../skills/plan-orchestrable-steps/SKILL.md | 4 +- packages/agents/content/skills/plan/SKILL.md | 6 +- .../content/skills/refine-plan/SKILL.md | 10 +- .../content/skills/review-branch/SKILL.md | 6 +- .../agents/content/skills/save-plan/SKILL.md | 6 +- .../skills/update-jira-ticket/SKILL.md | 4 +- .../skills/update-project-guidance/SKILL.md | 4 +- .../src/__tests__/spec-inlining.test.ts | 121 ++++++++++++++++++ 15 files changed, 240 insertions(+), 97 deletions(-) rename packages/agents/content/skills/{_data => _partials}/next-steps-after-plan.md (71%) rename packages/agents/content/skills/{_data => _partials}/next-steps-after-review.md (75%) create mode 100644 packages/agents/content/skills/_partials/option-format.md create mode 100644 packages/agents/src/__tests__/spec-inlining.test.ts diff --git a/packages/agents/content/skills/_data/recommendation-gradient.md b/packages/agents/content/skills/_data/recommendation-gradient.md index d8cb0e54..73caa207 100644 --- a/packages/agents/content/skills/_data/recommendation-gradient.md +++ b/packages/agents/content/skills/_data/recommendation-gradient.md @@ -1,9 +1,11 @@ # Recommendation gradient -> **Note for maintainers:** Several skill bodies — including `collaborate/SKILL.md`, `design-and-plan/SKILL.md`, and `refine-plan/SKILL.md` — contain pointers back to this file at their question-asking steps. Those pointers duplicate the universal rule in `AGENTS.md` _intentionally_: Agents follow behavioural rules more reliably when the directive sits near the action it governs. Do not remove these pointers during DRY-driven refactors — the redundancy is load-bearing. - For numbered option-style questions with 2 or more choices, mark each option with a strength gradient and a brief rationale. The gradient applies to every list with substantive tradeoffs, including templated next-steps menus and substantive binary choices. +The render contract comes first; the doctrine behind it follows. Skills that ask option-style questions carry the render contract inlined, so what they consult here is the doctrine. + + + ## Confirmation prompts vs. substantive binaries Reserve `👍🏼👎🏼` for confirmation prompts, where the agent has proposed a single action and the user's response is approve-or-redirect. "No" means "let's adjust or discuss," not a concrete alternative agent action. @@ -31,46 +33,11 @@ Same surface phrasing, two correct renderings: Both "yes" (extract) and "no" (inline) are concrete agent actions with their own tradeoffs. -## Markers - -| Marker | Label | When to use | -| ------ | -------------------- | ---------------------------------------------------------------------------------------- | -| ■■■ | strongly recommended | You'd actively push back if the developer picked otherwise. Reserve for clear-cut cases. | -| ■■□ | recommended | Your lean. Default level when you have a preference. | -| ■□□ | weakly recommended | A slight edge; mostly preference. | -| □□□ | not recommended | Clear drawbacks; included for completeness or to rule out explicitly. | - -If you have no preference (pure taste call), omit markers from every option. Don't explain the omission — the absence is the signal. - -Render markers as plain text, never inside backticks — backticks shrink the glyphs and hurt readability. - ## Ranking criteria Rank options on correctness — behavior, API quality, architectural soundness, testability, maintainability — and treat convenience considerations (effort, blast radius, consistency with existing code) as secondary. See [design priorities](./design-priorities.md) for the full rule and a before/after example. -## Format - -Marker, then option title and colon. Each pro (`➕`) and con (`➖`) goes on its own line, prefixed with 3 non-breaking-space characters (NBSP, U+00A0) for visual indent — regular ASCII spaces are commonly stripped or normalized in model output, so a visible character is needed to make the indent reliable. Apply this even when an option has only one pro or con. Lead with the strongest argument. Use semicolons between items and a period on the last. - -## Question identifiers - -When a single response contains 2+ option-style questions, prefix each question with an identifier so the user can reference answers unambiguously (e.g., "Q1: Option 2"). Default to `Q1`, `Q2`, etc. When the skill's underlying data already carries stable identifiers — for example, `refine-plan` presents questions tied to plan-review findings like `C1`, `X2` — use those identifiers in place of `Q1/Q2` so the cross-skill mapping is preserved. For a single option-style question, omit the identifier. - -## Examples - -Single question with markers: - -``` -Want me to: -1. ■□□ Use a single config file: -   ➕ minimal surface area; -   ➖ couples concerns. -2. ■■■ Split into two configs: -   ➕ separates lifecycle and runtime concerns; -   ➕ matches existing repo pattern. -3. □□□ Use three configs: -   ➖ over-decomposed for current scope. -``` +## Further examples Single question without markers (pure taste call): @@ -97,10 +64,3 @@ Multiple questions in one response (Q1/Q2 identifiers): 2. ■□□ Place in shared utility module:    ➕ reusable across packages. ``` - -## Don'ts - -- No tiebreaker text for equal-strength options. The developer picks the number. -- No partial marking. Once any option carries a marker, every option carries one. -- Cap at ■■□ unless you'd push back. Use ■■■ only when you'd actively object if the developer chose otherwise. -- No generic pros or cons. Each `➕` and `➖` must speak to the specific decision at hand (this plan, these findings, this design choice). Restatements of an option's inherent properties ("longer wall time", "structured review pass", "ships faster") are noise; the option's name and marker already communicate them. When no context-specific reasoning applies, omit pros and cons entirely — the marker alone is sufficient. diff --git a/packages/agents/content/skills/_data/next-steps-after-plan.md b/packages/agents/content/skills/_partials/next-steps-after-plan.md similarity index 71% rename from packages/agents/content/skills/_data/next-steps-after-plan.md rename to packages/agents/content/skills/_partials/next-steps-after-plan.md index e70182df..27896a09 100644 --- a/packages/agents/content/skills/_data/next-steps-after-plan.md +++ b/packages/agents/content/skills/_partials/next-steps-after-plan.md @@ -1,8 +1,6 @@ -# Next steps after plan +## Next-steps options -Standard next-steps block for skills that produce or refine an implementation plan. Skills reference this file to maintain a consistent format and recommendation logic. - -## Options +### Options | # | Emoji | Option | Description | | --- | ----- | ---------------------------------------- | ----------------------------------------------------------------------- | @@ -11,9 +9,9 @@ Standard next-steps block for skills that produce or refine an implementation pl | 3 | 🚀🔍 | Implement directly with follow-up review | Implement, then run a single end-of-work review pass as a separate step | | 4 | 🚀 | Implement directly | Implement without a follow-up review (reserved for trivial work) | -## Output format +### Output format -Present all four options as a numbered list in [recommendation-gradient](./recommendation-gradient.md) form. Each option carries a strength marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which option earns the strongest marker. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific plan presents a context-specific tradeoff bearing on which option fits (e.g., "plan introduces a new dependency boundary," "single module with no downstream effects"). Generic option properties ("structured review pass," "longer wall time") are noise and must be omitted; see the [recommendation gradient's don'ts](./recommendation-gradient.md#donts) for the rule. Include all known paths (plan, ticket) in each option line; omit paths that are not available in the current context. Use `~/`-relative paths where possible and absolute paths otherwise. +Present all four options as a numbered list per [option format](#option-format). Each option carries a strength marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which option earns the strongest marker. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific plan presents a context-specific tradeoff bearing on which option fits (e.g., "plan introduces a new dependency boundary," "single module with no downstream effects"). Generic option properties ("structured review pass," "longer wall time") are noise and must be omitted. Include all known paths (plan, ticket) in each option line; omit paths that are not available in the current context. Use `~/`-relative paths where possible and absolute paths otherwise. Options that invoke a skill include context-clearing guidance: @@ -45,7 +43,7 @@ Skill names for each option: - 🚀🔍 **Implement directly with follow-up review** -> no plan-time skill invocation; implement manually, then run `review-branch` (or `orchestrate-review`) as a separate post-implementation step - 🚀 **Implement directly** -> no skill invocation; implement manually or ask the agent to begin -## Recommendation rules +### Recommendation rules Select the recommended option by checking these rules in order and stopping at the first match. @@ -66,10 +64,10 @@ Select the recommended option by checking these rules in order and stopping at t 3. **Implement directly** — recommend instead of rule 2 when the work is trivial enough that a review pass would catch nothing meaningful (e.g., a typo fix, unused-import removal, single-file mechanical rename). Complexity levels 1–2 trivial only. 4. **Orchestrate** — all other cases (default). Cross-cutting changes, novel patterns, or work whose consequences ripple beyond the immediate change site fall here. -### Marker strengths +#### Marker strengths -The selected option carries the ■■□ marker in the rendered output. The other three options carry ■□□ by default. Reserve □□□ for an alternative with a clear drawback in the current context. Reserve ■■■ for the selected option only when you would actively push back against any other choice. See [recommendation-gradient markers](./recommendation-gradient.md#markers) for the full marker table and worked examples of the ■■■ and □□□ cases. +The selected option carries the ■■□ marker in the rendered output. The other three options carry ■□□ by default. Reserve □□□ for an alternative with a clear drawback in the current context. Reserve ■■■ for the selected option only when you would actively push back against any other choice. Each skill supplies its own recommendation context (e.g., whether the plan was developed interactively, whether a review just completed). Apply these rules using that context. -See [`scope-and-deferral.md`](scope-and-deferral.md) for the related decision on whether a finding warrants its own ticket. That decision (do now / batch later / separate ticket) composes with the recommendation rules above: The rules here pick the next-step _skill_; that reference governs whether work that surfaces alongside the current plan should spawn a new ticket or ship adjacent. +See [`scope-and-deferral.md`](../_data/scope-and-deferral.md) for the related decision on whether a finding warrants its own ticket. That decision (do now / batch later / separate ticket) composes with the recommendation rules above: The rules here pick the next-step _skill_; that reference governs whether work that surfaces alongside the current plan should spawn a new ticket or ship adjacent. diff --git a/packages/agents/content/skills/_data/next-steps-after-review.md b/packages/agents/content/skills/_partials/next-steps-after-review.md similarity index 75% rename from packages/agents/content/skills/_data/next-steps-after-review.md rename to packages/agents/content/skills/_partials/next-steps-after-review.md index 7e0be4c2..3cac9dd7 100644 --- a/packages/agents/content/skills/_data/next-steps-after-review.md +++ b/packages/agents/content/skills/_partials/next-steps-after-review.md @@ -1,25 +1,23 @@ -# Next steps after review - -Standard next-steps block for skills that produce a code review. Skills reference this file to maintain a consistent format and recommendation logic. +## Next-steps options The next-steps block has three independent sub-blocks. Each is shown only when its condition is met. If no condition is met, no next-steps block appears. Whatever combination of sub-blocks is shown, always wrap the output in a `Next steps:` header. Use `~/`-relative paths where possible and absolute paths otherwise. -## Deviations sub-block +### Deviations sub-block Shown when the ticket compliance section reports gaps (partial or unaddressed acceptance criteria) or unplanned work. -### Options +#### Options | # | Emoji | Option | Description | | --- | ----- | ------------- | ---------------------------------------------------- | | 1 | 📝 | Update ticket | Revise the ticket to match the actual implementation | | 2 | ⏭️ | Leave as-is | Accept the deviation without updating the ticket | -### Output format +#### Output format -Render the list in [recommendation-gradient](./recommendation-gradient.md) form. Each option carries a marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which markers apply. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific deviation presents a context-specific tradeoff (e.g., "the missing AC was load-bearing for downstream tests"). Generic restatements ("ships faster," "ticket drifts from reality") are noise and must be omitted; see the [recommendation gradient's don'ts](./recommendation-gradient.md#donts) for the rule. +Render the list per [option format](#option-format). Each option carries a marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which markers apply. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific deviation presents a context-specific tradeoff (e.g., "the missing AC was load-bearing for downstream tests"). Generic restatements ("ships faster," "ticket drifts from reality") are noise and must be omitted. Example (rendered for the recommendation case): @@ -34,18 +32,18 @@ Deviations from ticket: When the recommendation rules indicate no preference, omit markers from both options per the gradient's pure-taste-call form. -### Recommendation rules +#### Recommendation rules 1. **Recommend "Update ticket"** (■■□ on Update ticket, ■□□ on Leave as-is): acceptance criteria are missing or substantially different from what was implemented, OR significant unplanned work was done that should be captured. 2. **No recommendation** (omit markers from both options): deviations are minor and intentional (e.g., a criterion was addressed differently than originally described but the intent is met). The user decides. When uncertain, recommend updating the ticket. -## Source divergence sub-block +### Source divergence sub-block Shown when the consistency section of the review reports a `partial` or `severe` verdict. The option set varies by case (which spec source the implementation matches, drawn from the consistency-section table — see `review-branch/SKILL.md` § Specification consistency). -### Options +#### Options The base option pool is: @@ -59,9 +57,9 @@ The base option pool is: Each case renders three of these options; the specific options and their ordering are shown in the Output format section. -### Output format +#### Output format -Render the list in [recommendation-gradient](./recommendation-gradient.md) form. Each option carries a marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which option earns the strongest marker per case. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific divergence presents a context-specific tradeoff (e.g., "the diverging AC was load-bearing for adjacent work that has already shipped"). Generic restatements are noise and must be omitted; see the [recommendation gradient's don'ts](./recommendation-gradient.md#donts) for the rule. +Render the list per [option format](#option-format). Each option carries a marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which option earns the strongest marker per case. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific divergence presents a context-specific tradeoff (e.g., "the diverging AC was load-bearing for adjacent work that has already shipped"). Generic restatements are noise and must be omitted. Case 2 — implementation matches ticket; PR description is the stale source: @@ -98,7 +96,7 @@ Source divergence: Source-divergence options preserve conversation context because the divergence diagnosis from the review is the seed for whichever reconciliation action is taken. -### Recommendation rules +#### Recommendation rules In the typical flow, the ticket is written first and rarely revised, while the PR description describes the implementation as built. When the two diverge and the implementation matches one of them, the unmatched source is the stale one — update it to match reality. When the implementation matches neither, `design-and-plan` is the corrective: it handles both reconciliation cases (drift was intentional → ratify in the ticket; drift was unintended → plan against current reality with the existing code as material). @@ -110,15 +108,15 @@ Determine the case from the implementation column of the consistency-section tab | `🟠/🔴 ticket, 🟢 PR` on every divergent row | 🟠 `partial` | 3 | Update ticket | | `🟠/🔴 ticket, 🟠/🔴 PR` on any divergent row | 🔴 `severe` | 4 | Revisit design | -### Marker strengths +#### Marker strengths -The recommended option carries the ■■□ marker. Other options carry ■□□ by default. Reserve □□□ for an alternative with a clear drawback in the current context. Reserve ■■■ for the recommended option only when you would actively push back against any other choice. See [recommendation-gradient markers](./recommendation-gradient.md#markers) for the full marker table and worked examples of the ■■■ and □□□ cases. +The recommended option carries the ■■□ marker. Other options carry ■□□ by default. Reserve □□□ for an alternative with a clear drawback in the current context. Reserve ■■■ for the recommended option only when you would actively push back against any other choice. -## Findings sub-block +### Findings sub-block Shown when the review contains actionable findings (F, W, or T categories). -### Options +#### Options | # | Emoji | Option | Description | | --- | ----- | ---------------------------------------- | ------------------------------------------------------------------------------ | @@ -127,9 +125,9 @@ Shown when the review contains actionable findings (F, W, or T categories). | 3 | 🚀🔍 | Implement directly with follow-up review | Fix the findings, then run a single end-of-work review pass as a separate step | | 4 | 🚀 | Implement directly | Fix the findings without a follow-up review (reserved for trivial findings) | -### Output format +#### Output format -Render the list in [recommendation-gradient](./recommendation-gradient.md) form. Each option carries a marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which option earns the strongest marker. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific findings present a context-specific tradeoff bearing on which option fits (e.g., "fixes touch three modules with downstream effects"). Generic option properties ("structured review pass," "longer wall time") are noise and must be omitted; see the [recommendation gradient's don'ts](./recommendation-gradient.md#donts) for the rule. Include all known paths (ticket) in each option line; omit paths that are not available in the current context. +Render the list per [option format](#option-format). Each option carries a marker (■■■/■■□/■□□/□□□); the recommendation rules below determine which option earns the strongest marker. Pros and cons are omitted by default — add a `➕` or `➖` line only when the specific findings present a context-specific tradeoff bearing on which option fits (e.g., "fixes touch three modules with downstream effects"). Generic option properties ("structured review pass," "longer wall time") are noise and must be omitted. Include all known paths (ticket) in each option line; omit paths that are not available in the current context. Example (rendered for the default case, where the recommendation rules below select Orchestrate): @@ -158,26 +156,26 @@ Skill names for each option: - 🚀🔍 **Implement directly with follow-up review** -> no fix-time skill invocation; implement fixes manually, then run `review-branch` (or `orchestrate-review`) as a separate post-implementation step - 🚀 **Implement directly** -> no skill invocation; implement fixes manually or ask the agent to begin -### Recommendation rules +#### Recommendation rules Select the recommended option by checking these rules in order and stopping at the first match. -1. **Design and plan** — findings suggest the approach needs rethinking ([complexity level 4](complexity-classification.md)): architectural issues, fundamental design problems, or multiple FIXMEs that point to a flawed strategy. -2. **Implement directly with follow-up review** — findings are localized and a single end-of-work review pass would verify the fixes: single module/package, fixes are bounded, no downstream effects expected. The default for most actionable findings ([complexity level 3 bounded](complexity-classification.md), or non-trivial findings at levels 1–2). -3. **Implement directly** — findings are trivial enough that a re-review would catch nothing meaningful (e.g., a single typo fix, unused-import removal). [Complexity levels 1–2 trivial only](complexity-classification.md). -4. **Orchestrate** — all other cases (default). Findings are non-trivial AND cross-cutting ([complexity level 3 with downstream effects](complexity-classification.md), or a mix of warnings and TODOs that span multiple modules). +1. **Design and plan** — findings suggest the approach needs rethinking ([complexity level 4](../_data/complexity-classification.md)): architectural issues, fundamental design problems, or multiple FIXMEs that point to a flawed strategy. +2. **Implement directly with follow-up review** — findings are localized and a single end-of-work review pass would verify the fixes: single module/package, fixes are bounded, no downstream effects expected. The default for most actionable findings ([complexity level 3 bounded](../_data/complexity-classification.md), or non-trivial findings at levels 1–2). +3. **Implement directly** — findings are trivial enough that a re-review would catch nothing meaningful (e.g., a single typo fix, unused-import removal). [Complexity levels 1–2 trivial only](../_data/complexity-classification.md). +4. **Orchestrate** — all other cases (default). Findings are non-trivial AND cross-cutting ([complexity level 3 with downstream effects](../_data/complexity-classification.md), or a mix of warnings and TODOs that span multiple modules). -### Marker strengths +#### Marker strengths -The selected option carries the ■■□ marker in the rendered output. The other three options carry ■□□ by default. Reserve □□□ for an alternative with a clear drawback in the current context. Reserve ■■■ for the selected option only when you would actively push back against any other choice. See [recommendation-gradient markers](./recommendation-gradient.md#markers) for the full marker table and worked examples of the ■■■ and □□□ cases. +The selected option carries the ■■□ marker in the rendered output. The other three options carry ■□□ by default. Reserve □□□ for an alternative with a clear drawback in the current context. Reserve ■■■ for the selected option only when you would actively push back against any other choice. Complexity levels classify individual findings, but the recommendation applies to the collection. Multiple low-level findings that together indicate a design flaw may warrant a higher recommendation than any single finding's level suggests. When uncertain between two options, recommend the more thorough one. Each skill supplies its own recommendation context (e.g., finding counts and categories, severity of deviations). Apply these rules using that context. -See [`scope-and-deferral.md`](scope-and-deferral.md) for the cost-aware disposition that governs whether a deferred finding becomes a separate ticket, joins a batch, or ships as a drive-by. The recommendation rules above pick the _implementation skill_; that reference applies to any finding that the user defers rather than addressing immediately. +See [`scope-and-deferral.md`](../_data/scope-and-deferral.md) for the cost-aware disposition that governs whether a deferred finding becomes a separate ticket, joins a batch, or ships as a drive-by. The recommendation rules above pick the _implementation skill_; that reference applies to any finding that the user defers rather than addressing immediately. -## Combined output format +### Combined output format When multiple sub-blocks are shown, present them as separate sections within a single next-steps block. Ordering is Deviations → Source divergence → Actionable findings. The example below illustrates one possible arrangement; the recommendation rules in each sub-block determine which marker applies to each option: diff --git a/packages/agents/content/skills/_partials/option-format.md b/packages/agents/content/skills/_partials/option-format.md new file mode 100644 index 00000000..2d0da142 --- /dev/null +++ b/packages/agents/content/skills/_partials/option-format.md @@ -0,0 +1,36 @@ +## Option format + +Render every option-style question in this form — any numbered list of 2 or more choices with substantive tradeoffs, including templated next-steps menus and yes/no choices where both paths are concrete actions. Reserve `👍🏼👎🏼` for confirmation prompts, where a single action has been proposed and "no" means "let's adjust or discuss" rather than a concrete alternative. + +**Number every option** — `1.`, `2.`, `3.` The number is how the user selects. Never render the options as bullets or bare prose. + +**Mark every option with a strength marker.** The recommended option is the one carrying the strongest marker. There is no separate "I recommend option N" sentence, and a recommendation that does not match the strongest marker is a defect. + +| Marker | Label | When to use | +| ------ | -------------------- | ---------------------------------------------------------------------------------------- | +| ■■■ | strongly recommended | You'd actively push back if the developer picked otherwise. Reserve for clear-cut cases. | +| ■■□ | recommended | Your lean. Default level when you have a preference. | +| ■□□ | weakly recommended | A slight edge; mostly preference. | +| □□□ | not recommended | Clear drawbacks; included for completeness or to rule out explicitly. | + +Marking is all-or-none: once any option carries a marker, every option carries one. With no preference (a pure taste call), omit markers from every option and don't explain the omission — the absence is the signal. Cap at ■■□ unless you would push back; use ■■■ only when you would actively object to any other choice. Render markers as plain text, never inside backticks — backticks shrink the glyphs and hurt readability. + +**Format each option** as marker, then title, then a colon. Each pro (`➕`) and con (`➖`) goes on its own line, prefixed with 3 non-breaking-space characters (NBSP, U+00A0) for visual indent — regular ASCII spaces are commonly stripped or normalized in model output, so a visible character is needed to make the indent reliable. Lead with the strongest argument. Use semicolons between items and a period on the last. + +**Keep pros and cons context-specific.** Each `➕` and `➖` speaks to the decision at hand — this plan, these findings, this design choice. Restatements of an option's inherent properties ("longer wall time", "structured review pass", "ships faster") are noise; the option's name and marker already communicate them. Where no context-specific reasoning applies, omit pros and cons entirely — the marker alone is sufficient. Add no tiebreaker text for equal-strength options; the developer picks the number. + +**Identify each question** when a single response carries 2 or more option-style questions: prefix them `Q1`, `Q2`, and so on, so the user can reference answers unambiguously. Where the underlying data already carries stable identifiers — plan-review findings such as `C1` or `X2` — use those in place of `Q1`/`Q2`. For a single option-style question, omit the identifier. + +Example: + +``` +Want me to: +1. ■□□ Use a single config file: +   ➕ minimal surface area; +   ➖ couples concerns. +2. ■■■ Split into two configs: +   ➕ separates lifecycle and runtime concerns; +   ➕ matches existing repo pattern. +3. □□□ Use three configs: +   ➖ over-decomposed for current scope. +``` diff --git a/packages/agents/content/skills/collaborate/SKILL.md b/packages/agents/content/skills/collaborate/SKILL.md index 8dfd64c2..c03a0348 100644 --- a/packages/agents/content/skills/collaborate/SKILL.md +++ b/packages/agents/content/skills/collaborate/SKILL.md @@ -48,7 +48,7 @@ When you do ask, prefer forms the user can answer unambiguously: - **A confirmation prompt** (end with `👍🏼👎🏼`). The marker carries a fixed comprehension contract — a clear affirmation proceeds, a clear negation doesn't, anything else is conversation. Full spec in `AGENTS.md` under "Prompt formatting". (Reinforces the rule in `AGENTS.md` — intentional redundancy.) - **A numbered options list.** Include a "some other approach (describe)" option if alternatives should stay open. - - When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) + - When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) **Never use an interactive selector to pose the question.** Calling `{tool:AskUserQuestion}` (or any pop-up / arrow-key picker) cannot render the strength markers or pros and cons the gradient requires, so it silently discards the convention. Always write the choice as plain text in the message body. @@ -76,3 +76,5 @@ When you deem appropriate, proactively dispatch subagents to perform tasks. Good ## Skill improvement - When the user corrects the agent, or specifies a new desired behavior, that feedback is evidence for refining a skill, subagent, rulebook, general guidance, or helper. Invoke the `{skill:capture-feedback}` skill: it applies the immediate fix when there is something concrete and records a generalized `feedback` event — tagged `mistake` when existing guidance was missed — for a later refinement pass to mine. + + diff --git a/packages/agents/content/skills/design-and-plan/SKILL.md b/packages/agents/content/skills/design-and-plan/SKILL.md index 6de256d4..37dfcca7 100644 --- a/packages/agents/content/skills/design-and-plan/SKILL.md +++ b/packages/agents/content/skills/design-and-plan/SKILL.md @@ -68,7 +68,7 @@ Invoke the `{skill:assess-ticket}` skill with the resolved ticket source and mod - Success criteria and edge cases - Prefer multiple choice when possible - Only one question per message - - When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) + - When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) **Important:** Do not use `{tool:AskUserQuestion}` or any interactive selector (pop-up, arrow-key, structured-choice) for multiple-choice questions. Ask the question as plain text in the message body, with options as a numbered list. @@ -76,7 +76,7 @@ Invoke the `{skill:assess-ticket}` skill with the resolved ticket source and mod 1. **When the solution is obvious:** present the recommended approach directly. Don't manufacture alternatives for the sake of it. 2. **When the solution is not obvious:** propose 2-3 approaches with trade-offs. Lead with your recommendation and explain why. Rank options per [design priorities](../_data/design-priorities.md). - - When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) + - When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) 3. **Present the design** in sections scaled to complexity. Ask after each section whether it looks right. 4. **Get explicit approval** before proceeding. @@ -153,7 +153,7 @@ Design and plan complete: ``` -Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its options, output format, and recommendation rules exactly. Do not improvise the options. The plan was developed interactively with user approval at each stage — use this as recommendation context. Include both `{ticket_path}` and `{plan_path}` in each skill-invoking option line. +Follow the options, output format, and recommendation rules in [next-steps options](#next-steps-options) exactly. Do not improvise the options. The plan was developed interactively with user approval at each stage — use this as recommendation context. Include both `{ticket_path}` and `{plan_path}` in each skill-invoking option line. **STOP.** Do not invoke any other skill. Do not begin implementation. @@ -166,3 +166,7 @@ Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its o - **Scale to complexity**: A simple task gets a short design and a short plan - **Plan for engineers, not transcribers**: Communicate decisions, not ceremony - **The ticket is the contract**: If facts on the ground differ from the plan, the ticket's acceptance criteria are the source of truth + + + + diff --git a/packages/agents/content/skills/merge-pr/SKILL.md b/packages/agents/content/skills/merge-pr/SKILL.md index 823e02d0..1ab55a7c 100644 --- a/packages/agents/content/skills/merge-pr/SKILL.md +++ b/packages/agents/content/skills/merge-pr/SKILL.md @@ -128,7 +128,7 @@ Describe the accomplishment from the reader's standpoint. One short paragraph is If `scope.status` or `type.status` from step 3 is `ambiguous`, ask one question at a time before showing the final commit: - For each ambiguous dimension, present a numbered list of the dimension's `candidates` array, plus an "other (specify)" option. Ask the user to pick. If the candidates array is empty, ask open-ended. - - When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) + - When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) - After the user resolves each ambiguous dimension, re-render the title (step 5) with the now-concrete values. Then render the proposed merge to the user: @@ -182,3 +182,5 @@ The orchestrator never passes ambiguous-status dimensions or `prompt` sentinels - Local state is intentionally untouched after the merge. Branch deletion happens on the remote per the resolved decision; the local working copy and current branch are not modified. A separate skill may handle local cleanup later. The default `remote` mode deletes the remote branch via a post-merge `gh api -X DELETE` call (delegated to `merge-gh-pr`); `both` mode passes `--delete-branch` to `gh pr merge`, which is incompatible with worktree-based workflows — `gh pr merge --delete-branch` fails when the base branch is held by another worktree. - Never bypass branch protections. The orchestrator does not expose `--admin`; users who need that capability run `gh pr merge --admin` directly. - Never list automated checks (formatting, linting, typechecking, unit tests) in the merge body. They run automatically in CI. + + diff --git a/packages/agents/content/skills/plan-orchestrable-steps/SKILL.md b/packages/agents/content/skills/plan-orchestrable-steps/SKILL.md index d2e55e93..8f906b5f 100644 --- a/packages/agents/content/skills/plan-orchestrable-steps/SKILL.md +++ b/packages/agents/content/skills/plan-orchestrable-steps/SKILL.md @@ -55,7 +55,7 @@ Read `{artifact-dir}/orchestration-plan.json`. Present to the user: - **Dependency graph**: Which steps can run in parallel vs. which are sequential - **Risks**: Items that need user attention - **Questions**: Items the planner could not resolve from codebase analysis - - When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) + - When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) ### 4. User feedback loop @@ -119,3 +119,5 @@ When the user approves the plan: - All codebase exploration and plan generation is delegated to the planner agent — do not analyze code directly - The feedback loop is interactive — always wait for user input before resuming the planner - Do not proceed to orchestration — the user invokes `orchestrate-dev` separately when ready + + diff --git a/packages/agents/content/skills/plan/SKILL.md b/packages/agents/content/skills/plan/SKILL.md index 774919c4..9d1dc64b 100644 --- a/packages/agents/content/skills/plan/SKILL.md +++ b/packages/agents/content/skills/plan/SKILL.md @@ -80,5 +80,9 @@ Plan saved: {plan_path} ``` -Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its options, output format, and recommendation rules exactly. Do not improvise the options. For recommendation context, supply the source's design provenance from the resolve step — `plan` adds no interactive design phase of its own. Include both `{plan_path}` and `{ticket_source}` in each skill-invoking option line; omit the ticket path when the source was a free-form description rather than a ticket. +Follow the options, output format, and recommendation rules in [next-steps options](#next-steps-options) exactly. Do not improvise the options. For recommendation context, supply the source's design provenance from the resolve step — `plan` adds no interactive design phase of its own. Include both `{plan_path}` and `{ticket_source}` in each skill-invoking option line; omit the ticket path when the source was a free-form description rather than a ticket. + + + + diff --git a/packages/agents/content/skills/refine-plan/SKILL.md b/packages/agents/content/skills/refine-plan/SKILL.md index ce0571d5..53742e6b 100644 --- a/packages/agents/content/skills/refine-plan/SKILL.md +++ b/packages/agents/content/skills/refine-plan/SKILL.md @@ -87,12 +87,12 @@ Evaluate the finding counts: ``` - Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its options, output format, and recommendation rules exactly. Do not improvise the options. The plan was just reviewed with no issues — use this as recommendation context. Use `{plan_path}` (the original plan argument, not `{revision_output_path}` — no revised plan exists on this path) and `{ticket_source}` in each skill-invoking option line. + Follow the options, output format, and recommendation rules in [next-steps options](#next-steps-options) exactly. Do not improvise the options. The plan was just reviewed with no issues — use this as recommendation context. Use `{plan_path}` (the original plan argument, not `{revision_output_path}` — no revised plan exists on this path) and `{ticket_source}` in each skill-invoking option line. - **0 user questions** (UserQuestions = 0, AutoResolvable > 0): Skip user interaction. Proceed to step 5 with empty user answers. -- **User questions present** (UserQuestions > 0): Read the review artifact. Extract all findings from the "Decision gaps" section (these may be C or X findings -- the section is organized by resolution type, not finding category). Present each finding's question using the finding's ID (e.g., `C1`, `X2`) as the question identifier. When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) +- **User questions present** (UserQuestions > 0): Read the review artifact. Extract all findings from the "Decision gaps" section (these may be C or X findings -- the section is organized by resolution type, not finding category). Present each finding's question using the finding's ID (e.g., `C1`, `X2`) as the question identifier. When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) ``` The plan review identified {UserQuestions} question(s) that need your input: @@ -245,7 +245,7 @@ Plan refined: ``` -Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its options, output format, and recommendation rules exactly. Do not improvise the options. The plan was just reviewed. If the review surfaced significant scope changes or unresolved questions that led to a dramatic revision, the plan may warrant another refinement round; otherwise, either orchestration or direct implementation may apply depending on whether the work's consequences fit a single review pass. Use this as recommendation context. Include both `{revision_output_path}` (as the plan path) and `{ticket_source}` in each skill-invoking option line. +Follow the options, output format, and recommendation rules in [next-steps options](#next-steps-options) exactly. Do not improvise the options. The plan was just reviewed. If the review surfaced significant scope changes or unresolved questions that led to a dramatic revision, the plan may warrant another refinement round; otherwise, either orchestration or direct implementation may apply depending on whether the work's consequences fit a single review pass. Use this as recommendation context. Include both `{revision_output_path}` (as the plan path) and `{ticket_source}` in each skill-invoking option line. ## Edge cases @@ -258,3 +258,7 @@ Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its o - The user interaction step is conversational -- present questions as formatted text and wait for a free-form response. - This skill performs one review-and-revise round. Do not loop or iterate. - The original plan file is never modified. All output goes to new artifact files. + + + + diff --git a/packages/agents/content/skills/review-branch/SKILL.md b/packages/agents/content/skills/review-branch/SKILL.md index 7e7a4a28..2c235d8c 100644 --- a/packages/agents/content/skills/review-branch/SKILL.md +++ b/packages/agents/content/skills/review-branch/SKILL.md @@ -48,7 +48,7 @@ This skill is the canonical home of the shared review process. `review-pr` invok 8. **Assign a score** out of 10. 9. **Resolve frontmatter fields** before saving; see [Frontmatter resolution](#frontmatter-resolution). 10. **Save the review** per the [Saving](#saving) section. -11. **Present next steps**: After saving, present a next-steps prompt following [next-steps-after-review](../_data/next-steps-after-review.md). Supply recommendation context: finding counts and categories from the review, whether specification compliance gaps or unplanned work were identified, and the consistency verdict when the consistency section was rendered. The next-steps prompt is interactive output only and is not saved in the review artifact. +11. **Present next steps**: After saving, present a next-steps prompt following [next-steps options](#next-steps-options). Supply recommendation context: finding counts and categories from the review, whether specification compliance gaps or unplanned work were identified, and the consistency verdict when the consistency section was rendered. The next-steps prompt is interactive output only and is not saved in the review artifact. ## Frontmatter resolution @@ -277,3 +277,7 @@ The review is saved as a run artifact: `{timestamp}_reviewer_review.md` 3. Save: `{run-dir}/{timestamp}_reviewer_review.md` Each review is a separate artifact in the run directory. Do not append to existing files — the chronological sequence of files is the history. + + + + diff --git a/packages/agents/content/skills/save-plan/SKILL.md b/packages/agents/content/skills/save-plan/SKILL.md index 584244a0..28e1721e 100644 --- a/packages/agents/content/skills/save-plan/SKILL.md +++ b/packages/agents/content/skills/save-plan/SKILL.md @@ -47,5 +47,9 @@ Would you like to create or update a ticket for this work? If so, use the `{skil ``` -Read [next-steps-after-plan](../_data/next-steps-after-plan.md) and follow its options, output format, and recommendation rules exactly. Do not improvise the options. The plan was developed in conversation with user participation — use this as recommendation context. Omit the ticket path from option lines — no ticket path is available at completion time. +Follow the options, output format, and recommendation rules in [next-steps options](#next-steps-options) exactly. Do not improvise the options. The plan was developed in conversation with user participation — use this as recommendation context. Omit the ticket path from option lines — no ticket path is available at completion time. + + + + diff --git a/packages/agents/content/skills/update-jira-ticket/SKILL.md b/packages/agents/content/skills/update-jira-ticket/SKILL.md index fe14a9b7..a7157209 100644 --- a/packages/agents/content/skills/update-jira-ticket/SKILL.md +++ b/packages/agents/content/skills/update-jira-ticket/SKILL.md @@ -148,7 +148,7 @@ Use only if `INVALID_INPUT` still fires after the pre-flight checker returned `o #### 1. Surface the failure to the user -Do not create a probe ticket silently. Present the situation to the user and let them choose how to proceed. Use the [recommendation-gradient format](../_data/recommendation-gradient.md): +Do not create a probe ticket silently. Present the situation to the user and let them choose how to proceed. Use the [option format](#option-format): > Jira rejected this payload and the pre-flight checker found no known issues. This is likely a new failure class. How should I proceed? > @@ -226,3 +226,5 @@ If recorded failures distribute across truly **unknown classes** (no clear patte - Passing a file path to `description_html` / `comment_html`. - Retrying past the 4-retry cap. - Skipping the failure record after a recovery — this removes the evidence needed to extend the checker. + + diff --git a/packages/agents/content/skills/update-project-guidance/SKILL.md b/packages/agents/content/skills/update-project-guidance/SKILL.md index 964a536e..b21eddf8 100644 --- a/packages/agents/content/skills/update-project-guidance/SKILL.md +++ b/packages/agents/content/skills/update-project-guidance/SKILL.md @@ -74,7 +74,7 @@ For each finding, assign one of these scopes: - The distinction is **scope** (general vs project-specific), not nature (prescriptive vs descriptive). Project-specific conventions, commands, and architectural decisions all belong in PROJECT.md regardless of whether they are rules or facts. - Do not duplicate general guidance. If a project-specific convention _extends_ a general one, include only the delta. - When unsure about scope, ask the user — one question at a time, prefer multiple choice. - - When asking option-style questions, follow [`_data/recommendation-gradient.md`](../_data/recommendation-gradient.md). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) + - When asking option-style questions, follow [option format](#option-format). (Reinforces the rule in `AGENTS.md` — intentional redundancy.) - Content that is obvious from reading the code (e.g., "this project uses TypeScript") adds no value. Include only what would save an agent from a wrong assumption or a slow discovery. ### Phase 3: Generate @@ -168,3 +168,5 @@ Before presenting the draft, verify: - **Interactive** — ask when classification is unclear, but don't overwhelm with questions - **Portable** — this skill works in any repo that follows the `.agents/PROJECT.md` convention - **Honest about uncertainty** — if something might belong in general guidance, say so rather than silently including it + + diff --git a/packages/agents/src/__tests__/spec-inlining.test.ts b/packages/agents/src/__tests__/spec-inlining.test.ts new file mode 100644 index 00000000..72999d2b --- /dev/null +++ b/packages/agents/src/__tests__/spec-inlining.test.ts @@ -0,0 +1,121 @@ +import { existsSync } from 'node:fs'; +import { readdir, readFile } from 'node:fs/promises'; +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { expandIncludes } from '../lib/directive-expander.ts'; + +// Output-shaping specs — the option-format contract and the next-steps menus — must reach the agent inlined, not +// behind a runtime Markdown link. A link is an optional read at generation time, and the model will fill from its +// prior rather than take the hop, producing a block that looks right and is wrong. These tests assert the specs are +// present in each consumer's include-expanded body, which is what the install pipeline writes. +// +// The consumer lists are explicit rather than discovered from the include directives themselves: the failure this +// guards against is a consumer being *dropped*, and a discovered list would move with the bug. +const CONTENT_ROOT = new URL('../../content/', import.meta.url).pathname; +const SKILLS_ROOT = path.join(CONTENT_ROOT, 'skills'); + +// Written as an escape because a literal NBSP is invisible in source and is easily normalized to a plain space by +// an editor or formatter — the exact corruption the assertion below exists to catch. +const NBSP = '\u00A0'; + +interface Spec { + readonly name: string; + readonly heading: string; + /** Phrases that must survive distillation; each is a rule an improvised block has been observed to get wrong. */ + readonly rules: ReadonlyArray; +} + +const OPTION_FORMAT: Spec = { + name: 'option-format', + heading: '## Option format', + rules: [ + '**Number every option**', + 'a recommendation that does not match the strongest marker is a defect', + '| ■■■ | strongly recommended |', + `${NBSP.repeat(3)}➕`, + ], +}; + +const NEXT_STEPS_AFTER_PLAN: Spec = { + name: 'next-steps-after-plan', + heading: '## Next-steps options', + rules: ['| 3 | 🚀🔍 | Implement directly with follow-up review', '🎶 **Orchestrate** -> `orchestrate-dev`'], +}; + +const NEXT_STEPS_AFTER_REVIEW: Spec = { + name: 'next-steps-after-review', + heading: '## Next-steps options', + rules: ['### Source divergence sub-block', '### Combined output format'], +}; + +const CONSUMERS: ReadonlyArray<{ readonly slug: string; readonly specs: ReadonlyArray }> = [ + { slug: 'collaborate', specs: [OPTION_FORMAT] }, + { slug: 'design-and-plan', specs: [OPTION_FORMAT, NEXT_STEPS_AFTER_PLAN] }, + { slug: 'merge-pr', specs: [OPTION_FORMAT] }, + { slug: 'plan', specs: [OPTION_FORMAT, NEXT_STEPS_AFTER_PLAN] }, + { slug: 'plan-orchestrable-steps', specs: [OPTION_FORMAT] }, + { slug: 'refine-plan', specs: [OPTION_FORMAT, NEXT_STEPS_AFTER_PLAN] }, + { slug: 'review-branch', specs: [OPTION_FORMAT, NEXT_STEPS_AFTER_REVIEW] }, + { slug: 'save-plan', specs: [OPTION_FORMAT, NEXT_STEPS_AFTER_PLAN] }, + { slug: 'update-jira-ticket', specs: [OPTION_FORMAT] }, + { slug: 'update-project-guidance', specs: [OPTION_FORMAT] }, +]; + +/** The `_data` paths the specs were reached through before they were inlined. A surviving link is a missed consumer. */ +const RELOCATED_SPEC_LINKS: ReadonlyArray = [ + '_data/next-steps-after-plan.md', + '_data/next-steps-after-review.md', +]; + +/** Returns a skill's include-expanded `SKILL.md` — the body the install pipeline goes on to rewrite and write out. */ +async function expandSkill(slug: string): Promise { + return expandIncludes(path.join(SKILLS_ROOT, slug, 'SKILL.md'), CONTENT_ROOT); +} + +function countOccurrences(haystack: string, needle: string): number { + return haystack.split(needle).length - 1; +} + +describe('output-shaping spec inlining', () => { + describe.each(CONSUMERS)('$slug', ({ slug, specs }) => { + it.each(specs)('inlines the $name spec', async (spec) => { + const expanded = await expandSkill(slug); + expect(expanded).toContain(spec.heading); + for (const rule of spec.rules) { + expect(expanded).toContain(rule); + } + }); + + it('inlines each spec exactly once', async () => { + const expanded = await expandSkill(slug); + for (const heading of new Set(specs.map((spec) => spec.heading))) { + expect(countOccurrences(expanded, heading), `${slug} repeats "${heading}"`).toBe(1); + } + }); + }); + + it('no skill still links to a relocated spec', async () => { + const violations: Array = []; + for (const entry of await readdir(SKILLS_ROOT, { withFileTypes: true })) { + // `_`-prefixed entries are support directories; a directory with no `SKILL.md` (e.g. a bundled helper) is not + // a skill either. Neither can carry a spec link. + if (!entry.isDirectory() || entry.name.startsWith('_')) { + continue; + } + const skillPath = path.join(SKILLS_ROOT, entry.name, 'SKILL.md'); + if (!existsSync(skillPath)) { + continue; + } + const content = await readFile(skillPath, 'utf8'); + for (const link of RELOCATED_SPEC_LINKS) { + if (content.includes(link)) { + violations.push(`${entry.name}/SKILL.md -> ${link}`); + } + } + } + const message = `These specs are inlined now; replace each link with an in-file anchor:\n ${violations.join('\n ')}`; + expect(violations, message).toEqual([]); + }); +}); From 35b1b1612118ea786a8c2cec83af13f21e340e03 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 12 Jul 2026 10:22:41 -0700 Subject: [PATCH 3/5] agents|docs: Document the rule that decides _partials versus _data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contributors adding a shared Markdown fragment now have a stated rule for choosing where it lives: content an agent must reproduce is inlined at build time, and content it consults conditionally is reached by a link. The rule names the failure mode it prevents — an optional read the agent fills from its own prior instead of fetching — and records why the skill-local pointers to the option-format rules are deliberate rather than redundant. --- packages/agents/content/_partials/README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/agents/content/_partials/README.md b/packages/agents/content/_partials/README.md index 569091ed..e5d21854 100644 --- a/packages/agents/content/_partials/README.md +++ b/packages/agents/content/_partials/README.md @@ -4,6 +4,23 @@ Partials are reusable Markdown fragments shared across skills, subagents, and pl This README is the canonical reference for the partial system. The expander is implemented in `packages/agents/src/lib/directive-expander.ts`. +## Choosing a bucket + +Shared Markdown lives in one of two buckets. The choice is not stylistic — it decides whether the agent reliably sees the content. + +- **`_partials/` — content the agent must reproduce.** Output blocks, option menus, render formats, checklists it works through. Inlined at install time, so it is in context the moment the agent generates output. +- **`_data/` — content the agent consults conditionally.** Resolution tables, classification rubrics, doctrine references. Reached by a runtime Markdown link and read only when the situation calls for it. + +A runtime link is an optional read. Where the model already holds a strong prior for what the content looks like — and it does, for anything resembling a standard option menu or output block — it generates from that prior instead of taking the hop. Emphasis is not a remedy: a `` reading "follow its options and output format exactly; do not improvise" sat over one such link, and the block was improvised anyway. Never put must-reproduce content behind a runtime link. + +Inlining is not free — a partial's cost is paid by every consumer — so content the agent needs only sometimes stays in `_data/`. A spec that is partly must-reproduce and partly reference splits along that seam: the render contract becomes a partial, and the doctrine stays in `_data/` and includes the partial, so there is still one source of truth. + +Inline a spec **once per skill, as a section**, and point every use site at it with an in-file anchor (`[option format](#option-format)`). Anchor-only links pass through the link rewriter untouched. A skill with two use sites would otherwise carry the block twice, and a reference from inside a numbered procedure cannot absorb a long block inline. An in-file anchor costs nothing, because the content is already in context — the filesystem hop is the defect, not the pointer. + +### Skill-local pointers are load-bearing + +Several skill bodies — `collaborate`, `design-and-plan`, and `refine-plan` among them — carry a pointer to the option-format rules at their question-asking steps, duplicating the universal rule in `AGENTS.md`. That duplication is intentional: agents follow a behavioural rule more reliably when the directive sits near the action it governs. Do not remove these pointers during DRY-driven refactors — the redundancy is load-bearing. + ## Directive grammar Three include shapes are recognized. Each must occupy a full line, with optional leading and trailing whitespace. Inline directives inside prose or code spans are not expanded. From 018f15a78c9ac64e485b62aa6d2e954da4d0c5c8 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 12 Jul 2026 12:06:14 -0700 Subject: [PATCH 4/5] agents|fix: Restore the one-pro-or-con rule to the inlined option format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The option-format rules an agent renders now state that the per-line pro/con indent applies even when an option carries only a single pro or con — a rule the earlier distillation of those rules dropped. Without it, a one-item option could be rendered inline rather than on its own indented line, which is the formatting gap the rule was written to close. The rule is now pinned by the inlining assertions, so a future trim of the same clause fails the suite rather than shipping. --- packages/agents/content/skills/_partials/option-format.md | 2 +- packages/agents/src/__tests__/spec-inlining.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/agents/content/skills/_partials/option-format.md b/packages/agents/content/skills/_partials/option-format.md index 2d0da142..5cb7d568 100644 --- a/packages/agents/content/skills/_partials/option-format.md +++ b/packages/agents/content/skills/_partials/option-format.md @@ -15,7 +15,7 @@ Render every option-style question in this form — any numbered list of 2 or mo Marking is all-or-none: once any option carries a marker, every option carries one. With no preference (a pure taste call), omit markers from every option and don't explain the omission — the absence is the signal. Cap at ■■□ unless you would push back; use ■■■ only when you would actively object to any other choice. Render markers as plain text, never inside backticks — backticks shrink the glyphs and hurt readability. -**Format each option** as marker, then title, then a colon. Each pro (`➕`) and con (`➖`) goes on its own line, prefixed with 3 non-breaking-space characters (NBSP, U+00A0) for visual indent — regular ASCII spaces are commonly stripped or normalized in model output, so a visible character is needed to make the indent reliable. Lead with the strongest argument. Use semicolons between items and a period on the last. +**Format each option** as marker, then title, then a colon. Each pro (`➕`) and con (`➖`) goes on its own line, prefixed with 3 non-breaking-space characters (NBSP, U+00A0) for visual indent — regular ASCII spaces are commonly stripped or normalized in model output, so a visible character is needed to make the indent reliable. Apply this even when an option has only one pro or con. Lead with the strongest argument. Use semicolons between items and a period on the last. **Keep pros and cons context-specific.** Each `➕` and `➖` speaks to the decision at hand — this plan, these findings, this design choice. Restatements of an option's inherent properties ("longer wall time", "structured review pass", "ships faster") are noise; the option's name and marker already communicate them. Where no context-specific reasoning applies, omit pros and cons entirely — the marker alone is sufficient. Add no tiebreaker text for equal-strength options; the developer picks the number. diff --git a/packages/agents/src/__tests__/spec-inlining.test.ts b/packages/agents/src/__tests__/spec-inlining.test.ts index 72999d2b..6def3fd3 100644 --- a/packages/agents/src/__tests__/spec-inlining.test.ts +++ b/packages/agents/src/__tests__/spec-inlining.test.ts @@ -35,6 +35,7 @@ const OPTION_FORMAT: Spec = { 'a recommendation that does not match the strongest marker is a defect', '| ■■■ | strongly recommended |', `${NBSP.repeat(3)}➕`, + 'Apply this even when an option has only one pro or con.', ], }; From 1f63de0ed65fae959230a5a7d3b6c4af14ef7cc3 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Sun, 12 Jul 2026 12:06:23 -0700 Subject: [PATCH 5/5] agents|tests: Validate anchor fragments in the content link guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Markdown anchor that names no heading, or names two, now fails the build alongside a link whose file target is missing. Skills reach their inlined output-shaping specs through in-file anchors, so a renamed heading or a mistyped pointer would otherwise leave a dead locator repeated across every consuming skill with every gate green. Fragments are checked on cross-file links too, and fenced code blocks are excluded from both link and heading scanning — a fence illustrates output rather than declaring it, so a heading printed inside one is a sample, not an anchor target. --- .../__tests__/content-link-resolution.test.ts | 146 ++++++++++++++---- 1 file changed, 120 insertions(+), 26 deletions(-) diff --git a/packages/agents/src/__tests__/content-link-resolution.test.ts b/packages/agents/src/__tests__/content-link-resolution.test.ts index 8b12a413..66fc08a0 100644 --- a/packages/agents/src/__tests__/content-link-resolution.test.ts +++ b/packages/agents/src/__tests__/content-link-resolution.test.ts @@ -2,14 +2,19 @@ import { existsSync } from 'node:fs'; import { readdir } from 'node:fs/promises'; import path from 'node:path'; -import { describe, expect, it } from 'vitest'; +import { beforeAll, describe, expect, it } from 'vitest'; import { expandIncludes } from '../lib/directive-expander.ts'; -// A relative Markdown link in installable content is rewritten at install time by `rewriteMarkdownPaths`, which -// resolves it against the *host* file's directory — the skill or subagent the link renders into, not the partial it -// may have been authored in. A link whose target has moved or been deleted still installs cleanly, leaving the agent -// to follow a link to nothing. Resolving every link the way the installer does turns that into a build failure. +// A Markdown link in installable content is rewritten at install time by `rewriteMarkdownPaths`, which resolves a +// relative target against the *host* file's directory — the skill or subagent the link renders into, not the partial +// it may have been authored in — and leaves an anchor-only target untouched. A link whose target has moved or been +// deleted still installs cleanly, leaving the agent to follow a link to nothing. Resolving every link the way the +// installer does turns that into a build failure. +// +// Both halves of a link are checked: the file must exist, and any `#fragment` must name exactly one heading in the +// file it points into. Skills reach their inlined output-shaping specs through in-file anchors, so an unvalidated +// fragment is a dead locator repeated across every consumer. // // Host roots only. A `_partials/` file is never installed standalone, and its links are authored against the host that // inlines it — checking one in isolation would misresolve every `../` it carries. Include expansion below reaches them @@ -19,11 +24,26 @@ const HOST_ROOTS: ReadonlyArray = ['skills', 'subagents']; const CONTENT_ROOT = new URL('../../content/', import.meta.url).pathname; +const HEADING_REGEX = /^#{1,6}\s+(.+?)\s*$/gm; + const MARKDOWN_LINK_REGEX = /\[[^\]]*\]\(([^)]+)\)/g; +type Reason = 'ambiguous-anchor' | 'dead-anchor' | 'missing-file'; + interface Violation { readonly file: string; readonly target: string; + readonly reason: Reason; +} + +/** Counts each heading slug in a body, so a fragment matching two headings is reported rather than silently resolved. */ +function collectHeadingSlugs(body: string): ReadonlyMap { + const counts = new Map(); + for (const match of body.matchAll(HEADING_REGEX)) { + const slug = slugify(match[1] ?? ''); + counts.set(slug, (counts.get(slug) ?? 0) + 1); + } + return counts; } /** Recursively collects installable host `.md` files, skipping `_partials/` at any depth and dotfiles. */ @@ -48,23 +68,45 @@ async function findViolations(): Promise> { } hostFiles.sort(); + const headingsByFile = new Map>(); const violations: Array = []; + for (const hostFile of hostFiles) { - const expanded = await expandIncludes(hostFile, CONTENT_ROOT); - for (const match of expanded.matchAll(MARKDOWN_LINK_REGEX)) { + const body = stripFencedBlocks(await expandIncludes(hostFile, CONTENT_ROOT)); + const file = path.relative(CONTENT_ROOT, hostFile); + + for (const match of body.matchAll(MARKDOWN_LINK_REGEX)) { const target = match[1]; if (target === undefined || !isRelativeTarget(target)) { continue; } - const targetPath = target.split('#')[0]; - if (targetPath === undefined || targetPath === '') { + + const hashIndex = target.indexOf('#'); + const filePart = hashIndex === -1 ? target : target.slice(0, hashIndex); + const fragment = hashIndex === -1 ? '' : target.slice(hashIndex + 1); + + // An anchor-only target points into the host's own body; anything else names a file to resolve first. + const targetPath = filePart === '' ? hostFile : path.resolve(path.dirname(hostFile), filePart); + if (!existsSync(targetPath)) { + violations.push({ file, target, reason: 'missing-file' }); continue; } - if (!existsSync(path.resolve(path.dirname(hostFile), targetPath))) { - violations.push({ file: path.relative(CONTENT_ROOT, hostFile), target }); + + // Only Markdown carries headings; a fragment on any other target has nothing to resolve against. + if (fragment === '' || !targetPath.endsWith('.md')) { + continue; + } + + const headings = await readHeadingSlugs(targetPath, headingsByFile); + const matches = headings.get(fragment) ?? 0; + if (matches === 0) { + violations.push({ file, target, reason: 'dead-anchor' }); + } else if (matches > 1) { + violations.push({ file, target, reason: 'ambiguous-anchor' }); } } } + return violations; } @@ -73,27 +115,79 @@ function formatViolations(violations: ReadonlyArray): string { return ''; } const header = - `Found ${violations.length} relative Markdown link(s) whose target does not exist. Each is resolved against the ` + - `host file's directory, matching how \`rewriteMarkdownPaths\` resolves it at install time. A link authored in a ` + - `partial is reported against every host that inlines it, so fix the partial rather than the host.`; - const lines = violations.map((v) => ` ${v.file}: ${v.target}`); + `Found ${violations.length} unresolvable Markdown link(s). Each is resolved the way the install pipeline ` + + `resolves it: a relative path against the host file's directory, and a fragment against the headings of the ` + + `file it points into. A link authored in a partial is reported against every host that inlines it, so fix the ` + + `partial rather than the host.`; + const lines = violations.map((v) => ` [${v.reason}] ${v.file}: ${v.target}`); return [header, ...lines].join('\n'); } -/** Reports whether a link target is a relative path — the only form the install pipeline rewrites. */ +/** Reports whether a link target is one the install pipeline resolves — a relative path, an anchor, or both. */ function isRelativeTarget(target: string): boolean { - return !( - /^https?:\/\//.test(target) || - target.startsWith('/') || - target.startsWith('~') || - target.startsWith('#') || - target.startsWith('{') - ); + return !(/^https?:\/\//.test(target) || target.startsWith('/') || target.startsWith('~') || target.startsWith('{')); +} + +async function readHeadingSlugs( + file: string, + cache: Map>, +): Promise> { + const cached = cache.get(file); + if (cached !== undefined) { + return cached; + } + const slugs = collectHeadingSlugs(stripFencedBlocks(await expandIncludes(file, CONTENT_ROOT))); + cache.set(file, slugs); + return slugs; +} + +/** + * Derives a heading's anchor the way GitHub does: lowercase, drop everything but letters, numbers, spaces, and + * hyphens, then map each remaining space to a hyphen. Runs of spaces are preserved rather than collapsed — stripping + * punctuation between two spaces is what yields the double hyphen in an anchor such as `#finding-scheme-fwtrs--legacy-suffix`. + */ +function slugify(heading: string): string { + return heading + .trim() + .toLowerCase() + .replace(/[^\p{Letter}\p{Number}\s-]/gu, '') + .trim() + .replaceAll(' ', '-'); +} + +/** + * Blanks fenced code blocks. A fence illustrates output rather than declaring it, so a link or heading inside one is a + * sample, not a target: `review-branch` prints a `## Specification consistency` heading inside its output-format fence, + * which a naive scan would offer as a real anchor. + */ +function stripFencedBlocks(content: string): string { + let inFence = false; + return content + .split('\n') + .map((line) => { + if (/^\s*```/.test(line)) { + inFence = !inFence; + return ''; + } + return inFence ? '' : line; + }) + .join('\n'); } describe('installable-content link resolution', () => { - it('every relative Markdown link in an installable host resolves to a real file', async () => { - const violations = await findViolations(); - expect(violations, formatViolations(violations)).toEqual([]); + let violations: ReadonlyArray = []; + + beforeAll(async () => { + violations = await findViolations(); + }); + + it('every relative Markdown link in an installable host resolves to a real file', () => { + const missing = violations.filter((v) => v.reason === 'missing-file'); + expect(missing, formatViolations(missing)).toEqual([]); + }); + + it('every anchor fragment resolves to exactly one heading in the file it points into', () => { + const anchors = violations.filter((v) => v.reason !== 'missing-file'); + expect(anchors, formatViolations(anchors)).toEqual([]); }); });