From f8e5743b4c3b9176eb90d753de7da362175d9944 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Thu, 22 Oct 2026 09:49:27 -0500 Subject: [PATCH 01/32] feat(skills): shorter workflow skills that let the CLI do the exact work - Each workflow states its steps first and keeps rarely needed detail in references/. Shared rules are written once in references.ts. - Archive previews with `openspec archive --json`, asks about each warning and any unfinished artifact, then lets the CLI merge, validate, and move, or refuse and change nothing. - Explore keeps a living draft (scope, capabilities, requirements, open questions). Propose builds from the settled scope, reads the existing specs, checks what the code already does, and plans only the gap. - Spec guidance: one obligation per requirement, a scenario for each way the requested behavior fails with concrete values and no invented validation, no internal names, and compatible changes by default. - tasks.md ends with an "Update the specs" task and apply ends by offering to archive (#1968). `openspec context --json` returns project_context. Co-Authored-By: Claude Opus 5.5 --- .changeset/lean-skills-references.md | 5 + .gitignore | 1 + .../reference/schemas/spec-driven/index.md | 21 +- docs/commands.md | 30 +- docs/customization.md | 22 +- docs/supported-tools.md | 2 +- schemas/spec-driven/schema.yaml | 17 +- schemas/spec-driven/templates/spec.md | 4 + scripts/README.md | 4 + scripts/regen-parity-hashes.mjs | 9 +- src/commands/context.ts | 10 +- src/core/migration.ts | 39 + src/core/shared/index.ts | 3 + src/core/shared/skill-generation.ts | 86 ++ src/core/templates/skill-templates.ts | 2 +- src/core/templates/types.ts | 12 + src/core/templates/workflows/apply-change.ts | 202 +---- .../templates/workflows/archive-change.ts | 502 ++---------- src/core/templates/workflows/artifact-loop.ts | 52 ++ .../workflows/bulk-archive-change.ts | 749 ++---------------- .../templates/workflows/continue-change.ts | 246 +----- src/core/templates/workflows/explore.ts | 524 +++--------- src/core/templates/workflows/ff-change.ts | 233 +----- src/core/templates/workflows/new-change.ts | 144 +--- src/core/templates/workflows/onboard.ts | 382 +++++---- src/core/templates/workflows/project-root.ts | 37 - src/core/templates/workflows/propose.ts | 322 +------- src/core/templates/workflows/references.ts | 78 ++ .../templates/workflows/store-selection.ts | 7 - src/core/templates/workflows/sync-specs.ts | 561 ++----------- src/core/templates/workflows/update-change.ts | 134 +--- src/core/templates/workflows/verify-change.ts | 382 +++------ test/cli-e2e/basic.test.ts | 10 +- test/commands/context.test.ts | 26 + test/core/init.test.ts | 40 +- test/core/root-selection.test.ts | 4 +- test/core/shared/skill-generation.test.ts | 121 ++- .../templates/archive-task-progress.test.ts | 60 +- .../bulk-archive-existing-target.test.ts | 204 ++--- test/core/templates/explore.test.ts | 210 +++-- .../core/templates/glob-path-bounding.test.ts | 15 +- .../core/templates/list-json-contract.test.ts | 72 +- .../core/templates/project-root-guard.test.ts | 187 ++--- test/core/templates/propose.test.ts | 197 +++-- .../templates/skill-templates-parity.test.ts | 734 +++++------------ test/core/templates/spec-inventory.test.ts | 36 +- test/core/templates/update-change.test.ts | 20 +- .../verify-change-delta-operations.test.ts | 44 +- test/core/templates/verify-change.test.ts | 3 +- test/core/update-skill-tamper.test.ts | 31 +- test/core/update.test.ts | 26 +- test/helpers/skill-text.ts | 31 + test/utils/command-references.test.ts | 4 +- 53 files changed, 2147 insertions(+), 4750 deletions(-) create mode 100644 .changeset/lean-skills-references.md create mode 100644 src/core/templates/workflows/artifact-loop.ts delete mode 100644 src/core/templates/workflows/project-root.ts create mode 100644 src/core/templates/workflows/references.ts delete mode 100644 src/core/templates/workflows/store-selection.ts create mode 100644 test/helpers/skill-text.ts diff --git a/.changeset/lean-skills-references.md b/.changeset/lean-skills-references.md new file mode 100644 index 0000000000..9e77cb8bad --- /dev/null +++ b/.changeset/lean-skills-references.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": minor +--- + +Generated skills are about half the size. Each `SKILL.md` keeps the steps every run needs. Detail that only some runs need (store selection, an uninitialized project, example conversations, REMOVED and RENAMED checks, retiring a spec, the later onboarding phases) moves into a `references/` folder beside it, which the agent reads when a step sends it there. Command files carry the same text inline. Archive and bulk archive now let `openspec archive` merge, validate, and move each change instead of reproducing those steps by hand. `openspec context --json` now includes the project's validated `project_context`, so workflows no longer parse `config.yaml` themselves. Run `openspec update` to refresh installed skills. diff --git a/.gitignore b/.gitignore index 1fb5c4a26a..0afda935e5 100644 --- a/.gitignore +++ b/.gitignore @@ -166,3 +166,4 @@ opencode.json # Cursor .cursor/ +scripts/skill-eval/results/ diff --git a/docs-lab/reference/schemas/spec-driven/index.md b/docs-lab/reference/schemas/spec-driven/index.md index 5e9c1ea316..7a610d7022 100644 --- a/docs-lab/reference/schemas/spec-driven/index.md +++ b/docs-lab/reference/schemas/spec-driven/index.md @@ -99,7 +99,7 @@ Create the proposal document that establishes WHY this change is needed. Sections: - **Why**: 1-2 sentences on the problem or opportunity. What problem does this solve? Why now? -- **What Changes**: Bullet list of changes. Be specific about new capabilities, modifications, or removals. Mark breaking changes with **BREAKING**. +- **What Changes**: Bullet list of changes. Be specific about new capabilities, modifications, or removals. Mark breaking changes with **BREAKING**. Prefer the change that keeps existing behavior, data shapes, and tests working; when one must break, mark it **BREAKING** and say why it is needed. - **Capabilities**: Identify which specs will be created or modified: - **New Capabilities**: List capabilities being introduced. Each becomes a new `specs//spec.md`. Name each capability for a durable system behavior (for example, `user-auth`), not the work in this change (for example, `add-login-endpoint`). Choose a cohesive boundary that can own related requirements as the system evolves; avoid broad catch-all capabilities. Use kebab-case for path segments you introduce (e.g., `user-auth` or `identity/user-auth`) and follow the project's existing spec organization. - **Modified Capabilities**: List existing capabilities whose REQUIREMENTS are changing. Only include if spec-level behavior changes (not just implementation details). Each needs a delta spec file. Use the exact existing path under `openspec/specs/`. Leave empty if no requirement changes. @@ -159,6 +159,10 @@ The template the agent receives as the output format ([templates/spec.md](https: #### Scenario: - **WHEN** - **THEN** + +#### Scenario: +- **WHEN** +- **THEN** ``` ### Instructions @@ -219,6 +223,9 @@ Format requirements: - Each scenario: `#### Scenario: ` with WHEN/THEN format - **CRITICAL**: Scenarios MUST use exactly 4 hashtags (`####`). Using 3 hashtags or bullets will fail silently. - Every requirement MUST have at least one scenario. +- State one behavior per requirement: a single SHALL or MUST obligation, with no "and also". If a requirement bundles several obligations, split it into separate `### Requirement:` blocks. +- Cover the main success path, then add a scenario for each way the behavior can fail or surprise a reader (invalid input, a missing item, a refused action, a boundary value). A tester should be able to write one test per scenario, so use concrete example values (a specific date, an exact message) rather than placeholders like "a valid value". Describe how the requested behavior fails; do not add validation, errors, or limits the request did not ask for. +- Describe what a user or caller observes, not how the code does it: no internal function or class names, file paths, or private APIs in requirement text, unless they are themselves the public contract (for example, a documented CLI flag or API endpoint). - Keep each requirement's description (the text between `### Requirement:` and its first scenario) to 500 characters or fewer. `openspec validate` flags longer descriptions in ADDED requirements and in the main spec. This is a warning: normal validation still passes, but `openspec validate --strict` fails on it. When writing a new requirement, state one behavior per requirement: move examples and edge cases into scenarios, and split a requirement that covers several behaviors into separate `### Requirement:` blocks, each with its own scenarios. Under MODIFIED, keep the existing requirement block whole; never split, trim or rewrite existing text just to meet the length. Split an existing long requirement only when the user asks for it, in a change made for that purpose: under MODIFIED, keep its header and every scenario and cut its description down to one behavior without changing its meaning, then add each behavior you removed as its own ADDED requirement with its own scenarios. New capabilities only: the delta spec's first section is `## Purpose` - @@ -263,6 +270,10 @@ The system SHALL allow users to export their data in CSV format. - **WHEN** user clicks "Export" button - **THEN** system downloads a CSV file with all user data +#### Scenario: Nothing to export +- **WHEN** user with no data clicks "Export" button +- **THEN** system explains there is nothing to export and downloads no file + ## REMOVED Requirements ### Requirement: Legacy export @@ -383,6 +394,10 @@ Guidelines: - Each tracked task MUST be a checkbox: `- [ ] X.Y Task description` - Tasks should be small enough to complete in one session - Order tasks by dependency (what must be done first?) +- End with a tracked `## N. Update the specs` group: one task to sync this + change's delta specs into the main specs and verify `openspec validate + --specs` passes. It finishes before archive, so the main specs are + current as soon as the work is. - Track implementation and verification work that can be completed before archive. If the requested workflow includes archive or work that requires this change to be archived, preserve those steps as plain bullets in an @@ -415,6 +430,10 @@ Example: - [ ] 2.1 Implement data export function and verify the export test passes - [ ] 2.2 Add CSV formatting utilities and verify unit tests cover quoting and delimiters - [ ] 2.3 Document the export API in docs/export.md and verify the documented command runs as written + +## 3. Update the specs + +- [ ] 3.1 Sync this change's delta specs into the main specs and verify `openspec validate --specs` passes ``` When applicable, append workflow follow-up as plain bullets, for example: diff --git a/docs/commands.md b/docs/commands.md index ac64cb20e3..d9a1b740d7 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -525,10 +525,9 @@ Archive a completed change. Finalizes the change and moves it to the archive fol | `change-name` | No | Which change to archive (inferred from context if not provided) | **What it does:** -- Checks artifact completion status -- Checks task completion (warns if incomplete) -- Offers to sync delta specs if not already synced -- Moves change folder to `openspec/changes/archive/YYYY-MM-DD-/` +- Runs `openspec archive --json`. When nothing needs your go-ahead, that archives the change directly; otherwise it reports what needs approval and moves nothing +- Shows you any warnings (incomplete tasks or artifacts, main specs that will change) and asks before going on +- Runs `openspec archive --yes`, which merges the delta specs into the main specs, validates them, and moves the change folder to `openspec/changes/archive/YYYY-MM-DD-/`; if anything fails, the CLI refuses and nothing changes - Preserves all artifacts for audit trail **Example:** @@ -537,26 +536,19 @@ You: /opsx:archive AI: Archiving add-dark-mode... - Artifact status: - ✓ proposal.md exists - ✓ specs/ exists - ✓ design.md exists - ✓ tasks.md exists (8/8 tasks complete) - - Delta specs: Not yet synced - → Sync now? (recommended) + All 8 tasks are complete. Archiving will update 1 main spec + (specs/ui/spec.md: 1 requirement added). + → Archive and update the main specs? (recommended) You: Yes -AI: ✓ Synced specs to openspec/specs/ui/spec.md - ✓ Moved to openspec/changes/archive/2025-01-24-add-dark-mode/ - - Change archived successfully. +AI: ✓ Archived as openspec/changes/archive/2025-01-24-add-dark-mode/ + ✓ Main specs updated and validated (1 added) ``` **Tips:** -- Archive won't block on incomplete tasks, but will warn -- Delta specs can be synced during archive or beforehand +- Archive won't block on incomplete tasks, but will warn and ask +- Delta specs can be synced during archive or beforehand; archiving a change whose specs are already synced adds nothing twice - Archived changes are preserved for history - Use `/opsx:verify` first to catch issues @@ -581,7 +573,7 @@ Archive multiple completed changes at once. Handles spec conflicts between chang - Validates each change before archiving - Detects spec conflicts across changes - Resolves conflicts by checking what's actually implemented -- Archives in chronological order +- Archives each change with `openspec archive`, in chronological order when changes conflict **Example:** ``` diff --git a/docs/customization.md b/docs/customization.md index 03cca80a66..9eda6325bc 100644 --- a/docs/customization.md +++ b/docs/customization.md @@ -130,24 +130,22 @@ artifacts, or summaries unless the user separately requests that content. **Archive and spec-sync input safety:** -Archive, bulk archive, and standalone sync use +Archive and bulk archive hand the merge to `openspec archive`, which applies a +change's delta specs, validates the result, and moves the change, or refuses +without changing anything. Standalone sync merges by hand and uses `artifactPaths.specs.existingOutputPaths` from `openspec status --json` as the only delta-spec source. A schema without a `specs` artifact, or a change whose concrete output list is empty, has nothing to sync; other artifacts are not used to infer delta specs. -Before a semantic merge writes a main spec, the workflow consumes current +Before a hand merge writes a main spec, the sync workflow consumes current `openspec instructions specs --change --json` output. The returned -`specs` rules constrain only the main specs produced by that merge. Single archive -passes that snapshot into inline sync, standalone sync fetches it directly, and -bulk archive obtains every required snapshot before its first spec write. A -non-zero or invalid JSON archive/specs instruction response is a lookup failure, -not an empty input: the workflow stops before the affected spec write or change -move (for bulk archive, before any batch write or move). - -This configuration does not change archive execution phases, user prompts, -filesystem operations, semantic merge ownership, the direct `openspec archive` -command, or the structure and output of artifact `rules`. +`specs` rules constrain only the main specs produced by that merge. A non-zero +or invalid JSON response is a lookup failure, not an empty input: the workflow +stops before writing any main spec. + +This configuration does not change the direct `openspec archive` command or the +structure and output of artifact `rules`. ### Schema Resolution Order diff --git a/docs/supported-tools.md b/docs/supported-tools.md index f101819f88..b568a15b3b 100644 --- a/docs/supported-tools.md +++ b/docs/supported-tools.md @@ -6,7 +6,7 @@ OpenSpec works with many AI coding assistants. When you run `openspec init`, Ope For each selected tool, OpenSpec can install: -1. **Skills** (if delivery includes skills): `.../skills/openspec-*/SKILL.md` +1. **Skills** (if delivery includes skills): `.../skills/openspec-*/SKILL.md`, plus a `references/` folder beside it holding detail the skill reads only when a step needs it (for example, store selection or an uninitialized project) 2. **Commands** (if delivery includes commands): tool-specific `opsx-*` command files Codex is skills-only: OpenSpec installs `.agents/skills/openspec-*/SKILL.md` for Codex even when delivery is set to `commands`, and it does not generate Codex custom prompt files. Existing OpenSpec-managed skills under the legacy `.codex/skills` path are reconciled after their replacements are written; custom and divergent files are preserved. diff --git a/schemas/spec-driven/schema.yaml b/schemas/spec-driven/schema.yaml index 81336eb7fc..26071a0b3a 100644 --- a/schemas/spec-driven/schema.yaml +++ b/schemas/spec-driven/schema.yaml @@ -11,7 +11,7 @@ artifacts: Sections: - **Why**: 1-2 sentences on the problem or opportunity. What problem does this solve? Why now? - - **What Changes**: Bullet list of changes. Be specific about new capabilities, modifications, or removals. Mark breaking changes with **BREAKING**. + - **What Changes**: Bullet list of changes. Be specific about new capabilities, modifications, or removals. Mark breaking changes with **BREAKING**. Prefer the change that keeps existing behavior, data shapes, and tests working; when one must break, mark it **BREAKING** and say why it is needed. - **Capabilities**: Identify which specs will be created or modified: - **New Capabilities**: List capabilities being introduced. Each becomes a new `specs//spec.md`. Name each capability for a durable system behavior (for example, `user-auth`), not the work in this change (for example, `add-login-endpoint`). Choose a cohesive boundary that can own related requirements as the system evolves; avoid broad catch-all capabilities. Use kebab-case for path segments you introduce (e.g., `user-auth` or `identity/user-auth`) and follow the project's existing spec organization. - **Modified Capabilities**: List existing capabilities whose REQUIREMENTS are changing. Only include if spec-level behavior changes (not just implementation details). Each needs a delta spec file. Use the exact existing path under `openspec/specs/`. Leave empty if no requirement changes. @@ -105,6 +105,9 @@ artifacts: - Each scenario: `#### Scenario: ` with WHEN/THEN format - **CRITICAL**: Scenarios MUST use exactly 4 hashtags (`####`). Using 3 hashtags or bullets will fail silently. - Every requirement MUST have at least one scenario. + - State one behavior per requirement: a single SHALL or MUST obligation, with no "and also". If a requirement bundles several obligations, split it into separate `### Requirement:` blocks. + - Cover the main success path, then add a scenario for each way the behavior can fail or surprise a reader (invalid input, a missing item, a refused action, a boundary value). A tester should be able to write one test per scenario, so use concrete example values (a specific date, an exact message) rather than placeholders like "a valid value". Describe how the requested behavior fails; do not add validation, errors, or limits the request did not ask for. + - Describe what a user or caller observes, not how the code does it: no internal function or class names, file paths, or private APIs in requirement text, unless they are themselves the public contract (for example, a documented CLI flag or API endpoint). - Keep each requirement's description (the text between `### Requirement:` and its first scenario) to 500 characters or fewer. `openspec validate` flags longer descriptions in ADDED requirements and in the main spec. This is a warning: normal validation still passes, but `openspec validate --strict` fails on it. When writing a new requirement, state one behavior per requirement: move examples and edge cases into scenarios, and split a requirement that covers several behaviors into separate `### Requirement:` blocks, each with its own scenarios. Under MODIFIED, keep the existing requirement block whole; never split, trim or rewrite existing text just to meet the length. Split an existing long requirement only when the user asks for it, in a change made for that purpose: under MODIFIED, keep its header and every scenario and cut its description down to one behavior without changing its meaning, then add each behavior you removed as its own ADDED requirement with its own scenarios. New capabilities only: the delta spec's first section is `## Purpose` - @@ -149,6 +152,10 @@ artifacts: - **WHEN** user clicks "Export" button - **THEN** system downloads a CSV file with all user data + #### Scenario: Nothing to export + - **WHEN** user with no data clicks "Export" button + - **THEN** system explains there is nothing to export and downloads no file + ## REMOVED Requirements ### Requirement: Legacy export @@ -217,6 +224,10 @@ artifacts: - Each tracked task MUST be a checkbox: `- [ ] X.Y Task description` - Tasks should be small enough to complete in one session - Order tasks by dependency (what must be done first?) + - End with a tracked `## N. Update the specs` group: one task to sync this + change's delta specs into the main specs and verify `openspec validate + --specs` passes. It finishes before archive, so the main specs are + current as soon as the work is. - Track implementation and verification work that can be completed before archive. If the requested workflow includes archive or work that requires this change to be archived, preserve those steps as plain bullets in an @@ -249,6 +260,10 @@ artifacts: - [ ] 2.1 Implement data export function and verify the export test passes - [ ] 2.2 Add CSV formatting utilities and verify unit tests cover quoting and delimiters - [ ] 2.3 Document the export API in docs/export.md and verify the documented command runs as written + + ## 3. Update the specs + + - [ ] 3.1 Sync this change's delta specs into the main specs and verify `openspec validate --specs` passes ``` When applicable, append workflow follow-up as plain bullets, for example: diff --git a/schemas/spec-driven/templates/spec.md b/schemas/spec-driven/templates/spec.md index da74750674..da47243703 100644 --- a/schemas/spec-driven/templates/spec.md +++ b/schemas/spec-driven/templates/spec.md @@ -11,3 +11,7 @@ #### Scenario: - **WHEN** - **THEN** + +#### Scenario: +- **WHEN** +- **THEN** diff --git a/scripts/README.md b/scripts/README.md index 0fb25f7747..8fc3500811 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -70,3 +70,7 @@ test file mid-suite. ## pack-version-check.mjs Validates package version consistency before publishing. + +## skill-eval/ + +A/B test for generated skills: runs headless agent sessions on a fixture project and grades them with the OpenSpec CLI. See [skill-eval/README.md](skill-eval/README.md). diff --git a/scripts/regen-parity-hashes.mjs b/scripts/regen-parity-hashes.mjs index 917e20d2fb..7265665d92 100644 --- a/scripts/regen-parity-hashes.mjs +++ b/scripts/regen-parity-hashes.mjs @@ -77,7 +77,7 @@ if (srcMtime > distMtime) { } const templates = await import(distUrl('core/templates/skill-templates.js')); -const { getSkillTemplates, generateSkillContent } = await import( +const { getSkillTemplates, generateSkillFiles } = await import( distUrl('core/shared/skill-generation.js') ); @@ -85,6 +85,11 @@ const TEST_FILE = join(repoRoot, 'test/core/templates/skill-templates-parity.tes const sha256 = (value) => createHash('sha256').update(value).digest('hex'); +// A skill is SKILL.md plus its references; hash them together, each file +// prefixed by its path, so moving text between files moves the hash. +const serializeSkillFiles = (files) => + files.map((file) => `--- ${file.relativePath}\n${file.content}`).join('\n'); + // The generated-content hashes are keyed by skill directory. Read that mapping // from the same production helper the skills.sh generator uses, so a new // workflow never needs a second list kept in sync here. @@ -92,7 +97,7 @@ const PARITY_BASELINE = 'PARITY-BASELINE'; const contentByDir = new Map( getSkillTemplates().map(({ dirName, template }) => [ dirName, - sha256(generateSkillContent(template, PARITY_BASELINE)), + sha256(serializeSkillFiles(generateSkillFiles(template, PARITY_BASELINE))), ]) ); diff --git a/src/commands/context.ts b/src/commands/context.ts index fe93a2f50d..a0eea6050c 100644 --- a/src/commands/context.ts +++ b/src/commands/context.ts @@ -28,7 +28,7 @@ const FAILURE_PAYLOAD = { root: null, members: [] }; async function gatherWorkingSet( root: ResolvedOpenSpecRoot -): Promise<{ workingSet: WorkingSet; declaredReferenceCount: number }> { +): Promise<{ workingSet: WorkingSet; declaredReferenceCount: number; projectContext: string | null }> { const data = await gatherRelationshipData(root); // Reuse the 3.6 composition for member classification; the @@ -49,6 +49,7 @@ async function gatherWorkingSet( topLevelStatus: health.status, }), declaredReferenceCount: data.projectConfig?.references?.length ?? 0, + projectContext: data.projectConfig?.context ?? null, }; } @@ -172,7 +173,7 @@ export async function contextCommand(options: ContextOptions): Promise { return; } - const { workingSet, declaredReferenceCount } = await gatherWorkingSet(root); + const { workingSet, declaredReferenceCount, projectContext } = await gatherWorkingSet(root); if (options.json) { // The write runs FIRST: a write failure must leave stdout @@ -180,7 +181,10 @@ export async function contextCommand(options: ContextOptions): Promise { if (options.codeWorkspace) { writeCodeWorkspace(workingSet, options.codeWorkspace, options.force === true); } - printJson(workingSet); + // The root's `context:` config field, validated exactly as instructions + // load it, so a workflow can apply project background before any + // change exists. Null when the config has none or it is invalid. + printJson({ ...workingSet, project_context: projectContext }); } else { printHumanWorkingSet(workingSet, declaredReferenceCount); if (options.codeWorkspace) { diff --git a/src/core/migration.ts b/src/core/migration.ts index f1362bd611..2dc6c0a426 100644 --- a/src/core/migration.ts +++ b/src/core/migration.ts @@ -284,6 +284,7 @@ function migrateSkillDirs( fs.mkdirSync(destination, { recursive: true }); fs.renameSync(sourceSkill, destinationSkill); } + kept += migrateSkillReferences(projectPath, source, destination); // Anything the user left beside it stays under the legacy root. removeDirIfEmpty(source); moved++; @@ -295,6 +296,44 @@ function migrateSkillDirs( return { moved, kept }; } +/** + * Generated skills ship `references/.md` beside SKILL.md. Each one + * follows the same rule as SKILL.md: dropped when the destination already + * holds the same file, moved when it has none, and kept in place (and + * counted) when the two differ, since an edited copy is not ours to discard. + */ +function migrateSkillReferences(projectPath: string, source: string, destination: string): number { + const sourceRefs = path.join(source, 'references'); + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(sourceRefs, { withFileTypes: true }); + } catch { + return 0; + } + let kept = 0; + for (const entry of entries) { + if (!entry.isFile() || !entry.name.endsWith('.md')) continue; + const from = path.join(sourceRefs, entry.name); + const to = path.join(destination, 'references', entry.name); + if (!areProjectArtifacts(projectPath, from, to)) continue; + const disposition = classifyManagedFile(from, to); + try { + if (disposition === 'drop') { + fs.rmSync(from, { force: true }); + } else if (disposition === 'move') { + fs.mkdirSync(path.dirname(to), { recursive: true }); + fs.renameSync(from, to); + } else if (disposition === 'keep') { + kept++; + } + } catch { + kept++; + } + } + removeDirIfEmpty(sourceRefs); + return kept; +} + function migrateCommandFiles( projectPath: string, tool: AIToolOption, diff --git a/src/core/shared/index.ts b/src/core/shared/index.ts index fc7c1cd547..84bb18d5e7 100644 --- a/src/core/shared/index.ts +++ b/src/core/shared/index.ts @@ -27,6 +27,9 @@ export { getCommandTemplates, getCommandContents, generateSkillContent, + generateSkillFiles, + writeSkillFiles, + type GeneratedSkillFile, } from './skill-generation.js'; export { diff --git a/src/core/shared/skill-generation.ts b/src/core/shared/skill-generation.ts index bf3d307ca4..64a5aea0da 100644 --- a/src/core/shared/skill-generation.ts +++ b/src/core/shared/skill-generation.ts @@ -31,6 +31,8 @@ import { getOpsxProposeCommandTemplate, type SkillTemplate, } from '../templates/skill-templates.js'; +import * as fs from 'fs'; +import path from 'path'; import type { CommandContent } from '../command-generation/index.js'; import { assertWorkflowConditionalsResolved, @@ -38,6 +40,7 @@ import { } from '../templates/optional-workflow.js'; import { ALL_WORKFLOWS } from '../profiles.js'; import { OPENSPEC_CLI_ALLOWED_TOOLS } from './allowed-tools.js'; +import { FileSystemUtils } from '../../utils/file-system.js'; /** * The workflow set a template body is rendered against. @@ -103,6 +106,10 @@ export function getSkillTemplates(workflowFilter?: readonly string[]): SkillTemp template: { ...entry.template, instructions: resolveOptionalWorkflows(entry.template.instructions, installed), + references: entry.template.references?.map((reference) => ({ + ...reference, + content: resolveOptionalWorkflows(reference.content, installed), + })), }, })); } @@ -193,3 +200,82 @@ metadata: ${instructions} `; } + +/** One file of a generated skill, relative to the skill directory. */ +export interface GeneratedSkillFile { + relativePath: string; + content: string; +} + +const REFERENCE_PATH = /^references\/[a-z0-9][a-z0-9-]*\.md$/; + +/** + * Generates every file of a skill: SKILL.md plus its `references/` files. + * + * References get the same per-tool transform as SKILL.md, so a command + * reference reads the same way wherever the agent finds it. + * + * @param template - The skill template + * @param generatedByVersion - The OpenSpec version to embed in SKILL.md + * @param transformInstructions - Optional callback applied to every file body + */ +export function generateSkillFiles( + template: SkillTemplate, + generatedByVersion: string, + transformInstructions?: (instructions: string) => string +): GeneratedSkillFile[] { + const references = (template.references ?? []).map(({ path: relativePath, content }) => { + if (!REFERENCE_PATH.test(relativePath)) { + throw new Error( + `Skill '${template.name}' has an invalid reference path '${relativePath}'. Use references/.md.` + ); + } + const body = transformInstructions ? transformInstructions(content) : content; + assertWorkflowConditionalsResolved( + body, + `Skill '${template.name}' reference '${relativePath}' was generated without resolving its optional-workflow blocks` + ); + return { relativePath, content: body.endsWith('\n') ? body : `${body}\n` }; + }); + + return [ + { relativePath: 'SKILL.md', content: generateSkillContent(template, generatedByVersion, transformInstructions) }, + ...references, + ]; +} + +/** + * Writes a skill's files into `skillDir`, refusing any path outside `root`. + * + * The skill directory belongs to OpenSpec (removing a workflow deletes it + * whole), so a reference file a newer version no longer ships is removed + * rather than left for the agent to find. + */ +export async function writeSkillFiles( + root: string, + skillDir: string, + files: readonly GeneratedSkillFile[] +): Promise { + for (const file of files) { + const target = path.join(skillDir, file.relativePath); + FileSystemUtils.assertPathWithin(root, target); + await FileSystemUtils.writeFile(target, file.content); + } + + const referencesDir = path.join(skillDir, 'references'); + let entries: fs.Dirent[]; + try { + entries = await fs.promises.readdir(referencesDir, { withFileTypes: true }); + } catch { + return; + } + const kept = new Set(files.map((file) => file.relativePath)); + for (const entry of entries) { + const relativePath = `references/${entry.name}`; + if (entry.isFile() && entry.name.endsWith('.md') && !kept.has(relativePath)) { + const stale = path.join(referencesDir, entry.name); + FileSystemUtils.assertPathWithin(root, stale); + await fs.promises.rm(stale, { force: true }); + } + } +} diff --git a/src/core/templates/skill-templates.ts b/src/core/templates/skill-templates.ts index 041c0331c7..7b85bd51a8 100644 --- a/src/core/templates/skill-templates.ts +++ b/src/core/templates/skill-templates.ts @@ -4,7 +4,7 @@ * Compatibility facade that re-exports split workflow template modules. */ -export type { SkillTemplate, CommandTemplate } from './types.js'; +export type { SkillTemplate, SkillReference, CommandTemplate } from './types.js'; export { getExploreSkillTemplate, getOpsxExploreCommandTemplate } from './workflows/explore.js'; export { getNewChangeSkillTemplate, getOpsxNewCommandTemplate } from './workflows/new-change.js'; diff --git a/src/core/templates/types.ts b/src/core/templates/types.ts index 2c5ebb715c..b04d7c1fe7 100644 --- a/src/core/templates/types.ts +++ b/src/core/templates/types.ts @@ -2,10 +2,22 @@ * Core template types for skills and slash commands. */ +/** + * A file shipped beside SKILL.md, per the Agent Skills `references/` folder. + * The agent reads it only when SKILL.md points it there, so detail that most + * runs never need stays out of the always-read body. + */ +export interface SkillReference { + /** Path relative to the skill directory, always `references/.md`. */ + path: string; + content: string; +} + export interface SkillTemplate { name: string; description: string; instructions: string; + references?: SkillReference[]; license?: string; compatibility?: string; metadata?: Record; diff --git a/src/core/templates/workflows/apply-change.ts b/src/core/templates/workflows/apply-change.ts index 087911b0bd..7e6c0d44a0 100644 --- a/src/core/templates/workflows/apply-change.ts +++ b/src/core/templates/workflows/apply-change.ts @@ -1,13 +1,9 @@ /** - * Skill Template Workflow Modules - * - * This file is generated by splitting the legacy monolithic - * templates file into workflow-focused modules. + * Apply: implement a change's tasks, one checkbox at a time. */ import type { SkillTemplate, CommandTemplate } from '../types.js'; import { optionalWorkflow } from '../optional-workflow.js'; -import { STORE_SELECTION_GUIDANCE } from './store-selection.js'; -import { PROJECT_ROOT_GUARD } from './project-root.js'; +import { BEFORE_YOU_START, RUNTIME_INPUTS } from './references.js'; /** * `/opsx:continue` is not in the `core` profile, so the blocked-state handoff @@ -23,205 +19,55 @@ const BLOCKED_STATE_HANDOFF = optionalWorkflow( /** The archive handoff shown once every task is done. */ const ARCHIVE_HANDOFF = optionalWorkflow( 'archive', - 'You can archive this change with `/opsx:archive`.', - 'You can archive this change by running `openspec archive ""`.' + 'Then close it out: ask "Archive now? (recommended)", and on yes run the `/opsx:archive` workflow for it. If the user wants review first, leave it active.', + 'Then close it out: ask "Archive now? (recommended)", and on yes run `openspec archive "" --yes`. If the user wants review first, leave it active.' ); /** * The apply workflow instructions, authored once and rendered by both the - * skill and command surfaces. The surfaces are intentionally distinct, but - * they differ only in how they are invoked — the generation transformers - * rewrite the canonical `/opsx:` tokens per surface downstream (see - * command-references.ts). The instruction text itself is shared, so the two - * cannot silently drift. Should a surface ever need genuinely different - * wording, add a parameter here and pass it from that surface's template. + * skill and command surfaces. The surfaces differ only in how they are + * invoked; the generation transformers rewrite the canonical `/opsx:` + * tokens per surface (command-references.ts). */ export function getApplyInstructions(): string { - return `Implement tasks from an OpenSpec change. + return `Implement the tasks of an OpenSpec change, one checkbox at a time. -${STORE_SELECTION_GUIDANCE} - -${PROJECT_ROOT_GUARD} +${BEFORE_YOU_START} **Input**: Optionally specify a change name (e.g., \`/opsx:apply add-auth\`). If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. **Steps** -1. **Select the change** - - If a name is provided, use it. Otherwise: - - Infer from conversation context if the user mentioned a change - - Auto-select if only one active change exists - - If ambiguous, run \`openspec list --json\` to get available changes and ask the user to select one +0. **Check it is time to build.** If this same response already wrote a plan (propose, ff, new, or continue), stop: present the plan and let the user start apply in their next message, even if the request asked for both. - Always announce: "Using change: " and how to override (e.g., \`/opsx:apply \`). +1. **Pick the change.** Use the name given, the one from the conversation, or the only active change. Otherwise run \`openspec list --json\` and ask. Say "Using change: " and how to pick another (\`/opsx:apply \`). -2. **Check status to understand the schema** +2. **Read the plan.** \`\`\`bash openspec status --change "" --json - \`\`\` - Parse the JSON to understand: - - \`schemaName\`: The workflow being used (e.g., "spec-driven") - - \`planningHome\`, \`changeRoot\`, and \`actionContext\`: planning scope and edit constraints - - Which artifact contains the tasks (typically "tasks" for spec-driven, check status for others) - -3. **Get apply instructions** - - \`\`\`bash openspec instructions apply --change "" --json \`\`\` + Status gives the \`schemaName\`, the \`planningHome\` and \`changeRoot\` paths, and \`actionContext\` (where code may be edited; for a store, where the implementation lives). The apply instructions give the \`state\`, the \`instruction\`, progress, each task with its \`sourcePath\` and \`line\`, \`contextFiles\` (artifact ID -> array of concrete file paths), and \`missingArtifacts\`. ${RUNTIME_INPUTS} - This returns: - - \`contextFiles\`: artifact ID -> array of concrete file paths (varies by schema - could be proposal/specs/design/tasks or spec/tests/implementation/docs) - - Progress (total, complete, remaining) - - Task list with status, source path, and source line - - Dynamic instruction based on current state - - Optional \`context\`: current required project instruction input from the selected root - - Optional \`operationGuidance\`: current advisory guidance for apply - - \`missingArtifacts\` (when present): required artifact ids with no output - - **Handle states:** - - If \`state: "blocked"\`: show the message and pause implementation. - - If \`missingArtifacts\` is non-empty: ${BLOCKED_STATE_HANDOFF} - - Otherwise, follow the CLI instruction to create or repair the schema-configured tracking file from existing planning artifacts. Do not assume another artifact is ready or start implementation while blocked. - - If \`state: "all_done"\`: report that all tracked tasks are complete and suggest review or verification as appropriate before archiving - - Otherwise: proceed to implementation - - Treat \`context\` as a required prompt-level input. Read and consider it, and - apply relevant project facts, conventions, and constraints while implementing. - Treat \`operationGuidance\` as optional additive advice. Read and consider every - entry, and follow entries that are applicable and compatible with the built-in - workflow. - - Keep both fields separate from CLI-returned state, missing artifacts, tasks, - progress, \`contextFiles\`, and the built-in \`instruction\`. They are not - evidence of task completion, do not replace the built-in instruction, and do - not permit bypassing a blocked state. If context conflicts with the built-in - instruction, an explicit user choice, or a CLI-controlled value, report the - conflict and preserve the controlling value. If guidance is inapplicable or - conflicts with those controlling inputs, do not follow it and explain why. - These are prompt-level behavior contracts, not enforceable checks. - -4. **Read context files** - - Read every file path listed under \`contextFiles\` from the apply instructions output. - The files depend on the schema being used: - - **spec-driven**: proposal, specs, design, tasks - - Other schemas: follow the contextFiles from CLI output - - Do not copy \`context\` or \`operationGuidance\` verbatim into implementation - files or planning artifacts unless the user separately asks for that content. - -5. **Show current progress** - - Display: - - Schema being used - - Progress: "N/M tasks complete" - - Remaining tasks overview - - Dynamic instruction from CLI - -6. **Implement tasks (loop until done or blocked)** - - For each pending task: - - Show which task is being worked on - - Make the code changes required - - Keep changes minimal and focused - - Before editing, confirm the checkbox at the returned \`sourcePath\` and \`line\` still matches the task description; if it does not, rerun the apply instructions and use the refreshed location - - Mark the task complete at its returned \`sourcePath\` and \`line\`: \`- [ ]\` → \`- [x]\` - - Rerun the apply instructions and confirm that task is now done and progress changed - - Continue to next task - - **Pause if:** - - Task is unclear → ask for clarification - - Implementation reveals a design issue → suggest updating artifacts - - A task needs work beyond what the spec and tasks describe, or you are tempted to drop, narrow, defer, or accept exceptions to specified behavior to make it fit → surface the added scope and ask; do not absorb it silently - - Error or blocker encountered → report and wait for guidance - - User interrupts - -7. **On completion or pause, show status** - - Display: - - Tasks completed this session - - Overall progress: "N/M tasks complete" - - If all done: report that tracked tasks are complete and suggest review or verification as appropriate before archiving - - If paused: explain why and wait for guidance - -**Output During Implementation** - -\`\`\` -## Implementing: (schema: ) - -Working on task 3/7: -[...implementation happening...] -✓ Task complete - -Working on task 4/7: -[...implementation happening...] -✓ Task complete -\`\`\` - -**Output On Completion** - -\`\`\` -## Implementation Complete - -**Change:** -**Schema:** -**Progress:** 7/7 tasks complete ✓ - -### Completed This Session -- [x] Task 1 -- [x] Task 2 -... - -All tracked tasks are complete. Review or verify the change as appropriate -before archiving. ${ARCHIVE_HANDOFF} -\`\`\` - -**Output On Pause (Issue Encountered)** + - If \`state: "blocked"\`: show the message and pause implementation. If \`missingArtifacts\` is non-empty, ${BLOCKED_STATE_HANDOFF} Otherwise follow the CLI instruction to repair the tracking file. Do not assume another artifact is ready or start implementation while blocked. + - If \`state: "all_done"\`: report that all tracked tasks are complete and suggest review or verification as appropriate before archiving. -\`\`\` -## Implementation Paused - -**Change:** -**Schema:** -**Progress:** 4/7 tasks complete +3. **Read every file in \`contextFiles\`.** Use contextFiles from CLI output, don't assume specific file names. Show the schema, "N/M tasks complete", and what remains. -### Issue Encountered - +4. **Work through the tasks.** For each pending task: say which one, make the smallest change that fully does it, then mark it \`- [ ]\` → \`- [x]\` at its \`sourcePath\` and \`line\` (confirm that line still matches the task first; if not, rerun the apply instructions). Rerun the apply instructions to confirm progress moved. -**Options:** -1.