diff --git a/.changeset/quiet-proposals-stop.md b/.changeset/quiet-proposals-stop.md new file mode 100644 index 0000000000..b6d973e91f --- /dev/null +++ b/.changeset/quiet-proposals-stop.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Keep the propose workflow focused on planning, clarify material ambiguities before creating a change, and hand implementation off to the apply workflow. diff --git a/skills/openspec-propose/SKILL.md b/skills/openspec-propose/SKILL.md index 327d3f65d5..3b852b58d3 100644 --- a/skills/openspec-propose/SKILL.md +++ b/skills/openspec-propose/SKILL.md @@ -11,13 +11,15 @@ metadata: Propose a new change - create the change and generate all artifacts in one step. +**Planning boundary**: This workflow creates planning artifacts only. The user request that selected or triggered this workflow authorizes planning only, even if it asks to build or fix something. Do not edit project code. After the planning artifacts are complete, stop. Do not start implementation in the same response, even if the initial request asks for it. Wait for a new user request after the artifacts are presented; then start the apply workflow. + I'll create a change with the artifacts your schema defines. With the default spec-driven schema that is: - proposal.md (what & why) - `specs//spec.md` (what the system must do - a delta, not the main spec) - design.md (how) - tasks.md (implementation steps) -When ready to implement, run /openspec-apply-change +When the user is ready to implement, they must start the apply workflow explicitly. --- @@ -27,15 +29,17 @@ When ready to implement, run /openspec-apply-change **Steps** -1. **If no clear input provided, ask what they want to build** +1. **Understand the request and clarify material ambiguity** - Ask the user (open-ended, no preset options): + If no clear input is provided, ask the user (open-ended, no preset options): > "What change do you want to work on? Describe what you want to build or fix." From their description, derive a kebab-case name (e.g., "add user authentication" → `add-user-auth`). **IMPORTANT**: Do NOT proceed without understanding what the user wants to build. + If the request contains ambiguity that would materially affect scope, externally observable behavior, compatibility, or acceptance criteria, ask the user before creating the change. For minor details, make a reasonable assumption and record it in the planning artifacts. + 2. **Create the change directory** ```bash openspec new change "" @@ -101,7 +105,7 @@ After completing all artifacts, summarize: - Change name and location - List of artifacts created with brief descriptions, plus any conditional artifact you skipped and why - What's ready: "All artifacts needed for implementation are ready." -- Prompt: "Run `/openspec-apply-change` or ask me to implement to start working on the tasks." +- Prompt: "The artifacts are ready for review. When you are ready, run `/openspec-apply-change` or ask me to apply this change." **Artifact Creation Guidelines** @@ -115,8 +119,9 @@ After completing all artifacts, summarize: - These guide what you write, but should never appear in the output **Guardrails** +- The request that invoked this workflow authorizes planning only. Any implementation or apply instruction in that request does not carry forward. Do NOT implement the change, start the apply workflow, or edit project code during this workflow. After presenting the artifacts, stop and wait for a new user request to start the apply workflow - Create every artifact the apply phase transitively depends on, not just the ids listed in `apply.requires` - Always read dependency artifacts before creating a new one - re-read from disk, not from conversation memory (files may have changed since you last saw them) -- If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum +- Ask about ambiguities that would materially change scope, externally observable behavior, compatibility, or acceptance criteria; for minor details, make reasonable assumptions and record them - If a change with that name already exists, ask if user wants to continue it or create a new one - Verify each artifact file exists after writing before proceeding to next diff --git a/src/core/templates/workflows/propose.ts b/src/core/templates/workflows/propose.ts index f3258b81fb..c8f54c14a1 100644 --- a/src/core/templates/workflows/propose.ts +++ b/src/core/templates/workflows/propose.ts @@ -13,13 +13,15 @@ export function getOpsxProposeSkillTemplate(): SkillTemplate { description: 'Propose a new change with all artifacts generated in one step. Use when the user wants to quickly describe what they want to build and get a complete proposal with design, specs, and tasks ready for implementation.', instructions: `Propose a new change - create the change and generate all artifacts in one step. +**Planning boundary**: This workflow creates planning artifacts only. The user request that selected or triggered this workflow authorizes planning only, even if it asks to build or fix something. Do not edit project code. After the planning artifacts are complete, stop. Do not start implementation in the same response, even if the initial request asks for it. Wait for a new user request after the artifacts are presented; then start the apply workflow. + I'll create a change with the artifacts your schema defines. With the default spec-driven schema that is: - proposal.md (what & why) - \`specs//spec.md\` (what the system must do - a delta, not the main spec) - design.md (how) - tasks.md (implementation steps) -When ready to implement, run /opsx:apply +When the user is ready to implement, they must start the apply workflow explicitly. --- @@ -29,15 +31,17 @@ ${STORE_SELECTION_GUIDANCE} **Steps** -1. **If no clear input provided, ask what they want to build** +1. **Understand the request and clarify material ambiguity** - Ask the user (open-ended, no preset options): + If no clear input is provided, ask the user (open-ended, no preset options): > "What change do you want to work on? Describe what you want to build or fix." From their description, derive a kebab-case name (e.g., "add user authentication" → \`add-user-auth\`). **IMPORTANT**: Do NOT proceed without understanding what the user wants to build. + If the request contains ambiguity that would materially affect scope, externally observable behavior, compatibility, or acceptance criteria, ask the user before creating the change. For minor details, make a reasonable assumption and record it in the planning artifacts. + 2. **Create the change directory** \`\`\`bash openspec new change "" @@ -103,7 +107,7 @@ After completing all artifacts, summarize: - Change name and location - List of artifacts created with brief descriptions, plus any conditional artifact you skipped and why - What's ready: "All artifacts needed for implementation are ready." -- Prompt: "Run \`/opsx:apply\` or ask me to implement to start working on the tasks." +- Prompt: "The artifacts are ready for review. When you are ready, run \`/opsx:apply\` or ask me to apply this change." **Artifact Creation Guidelines** @@ -117,9 +121,10 @@ After completing all artifacts, summarize: - These guide what you write, but should never appear in the output **Guardrails** +- The request that invoked this workflow authorizes planning only. Any implementation or apply instruction in that request does not carry forward. Do NOT implement the change, start the apply workflow, or edit project code during this workflow. After presenting the artifacts, stop and wait for a new user request to start the apply workflow - Create every artifact the apply phase transitively depends on, not just the ids listed in \`apply.requires\` - Always read dependency artifacts before creating a new one - re-read from disk, not from conversation memory (files may have changed since you last saw them) -- If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum +- Ask about ambiguities that would materially change scope, externally observable behavior, compatibility, or acceptance criteria; for minor details, make reasonable assumptions and record them - If a change with that name already exists, ask if user wants to continue it or create a new one - Verify each artifact file exists after writing before proceeding to next`, license: 'MIT', @@ -136,13 +141,15 @@ export function getOpsxProposeCommandTemplate(): CommandTemplate { tags: ['workflow', 'artifacts', 'experimental'], content: `Propose a new change - create the change and generate all artifacts in one step. +**Planning boundary**: This workflow creates planning artifacts only. The user request that selected or triggered this workflow authorizes planning only, even if it asks to build or fix something. Do not edit project code. After the planning artifacts are complete, stop. Do not start implementation in the same response, even if the initial request asks for it. Wait for a new user request after the artifacts are presented; then start the apply workflow. + I'll create a change with the artifacts your schema defines. With the default spec-driven schema that is: - proposal.md (what & why) - \`specs//spec.md\` (what the system must do - a delta, not the main spec) - design.md (how) - tasks.md (implementation steps) -When ready to implement, run /opsx:apply +When the user is ready to implement, they must start the apply workflow explicitly. --- @@ -152,15 +159,17 @@ ${STORE_SELECTION_GUIDANCE} **Steps** -1. **If no input provided, ask what they want to build** +1. **Understand the request and clarify material ambiguity** - Ask the user (open-ended, no preset options): + If no input is provided, ask the user (open-ended, no preset options): > "What change do you want to work on? Describe what you want to build or fix." From their description, derive a kebab-case name (e.g., "add user authentication" → \`add-user-auth\`). **IMPORTANT**: Do NOT proceed without understanding what the user wants to build. + If the request contains ambiguity that would materially affect scope, externally observable behavior, compatibility, or acceptance criteria, ask the user before creating the change. For minor details, make a reasonable assumption and record it in the planning artifacts. + 2. **Create the change directory** \`\`\`bash openspec new change "" @@ -226,7 +235,7 @@ After completing all artifacts, summarize: - Change name and location - List of artifacts created with brief descriptions, plus any conditional artifact you skipped and why - What's ready: "All artifacts needed for implementation are ready." -- Prompt: "Run \`/opsx:apply\` to start implementing." +- Prompt: "The artifacts are ready for review. When you are ready, run \`/opsx:apply\`." **Artifact Creation Guidelines** @@ -240,9 +249,10 @@ After completing all artifacts, summarize: - These guide what you write, but should never appear in the output **Guardrails** +- The request that invoked this workflow authorizes planning only. Any implementation or apply instruction in that request does not carry forward. Do NOT implement the change, start the apply workflow, or edit project code during this workflow. After presenting the artifacts, stop and wait for a new user request to start the apply workflow - Create every artifact the apply phase transitively depends on, not just the ids listed in \`apply.requires\` - Always read dependency artifacts before creating a new one - re-read from disk, not from conversation memory (files may have changed since you last saw them) -- If context is critically unclear, ask the user - but prefer making reasonable decisions to keep momentum +- Ask about ambiguities that would materially change scope, externally observable behavior, compatibility, or acceptance criteria; for minor details, make reasonable assumptions and record them - If a change with that name already exists, ask if user wants to continue it or create a new one - Verify each artifact file exists after writing before proceeding to next` }; diff --git a/test/core/init.test.ts b/test/core/init.test.ts index a9dc5597f4..308d9a8b74 100644 --- a/test/core/init.test.ts +++ b/test/core/init.test.ts @@ -521,6 +521,47 @@ describe('InitCommand', () => { expect(await fileExists(cursorSkill)).toBe(true); }); + it('should deliver the propose boundary to tools named in the linked reports', async () => { + saveGlobalConfig({ + featureFlags: {}, + profile: 'core', + delivery: 'both', + }); + + const initCommand = new InitCommand({ + tools: 'factory,cursor,kilocode,pi,codex', + force: true, + }); + await initCommand.execute(testDir); + + const proposeFiles = [ + path.join(testDir, '.factory', 'commands', 'opsx-propose.md'), + path.join(testDir, '.cursor', 'commands', 'opsx-propose.md'), + path.join(testDir, '.kilocode', 'workflows', 'opsx-propose.md'), + path.join(testDir, '.pi', 'prompts', 'opsx-propose.md'), + path.join(testDir, '.codex', 'skills', 'openspec-propose', 'SKILL.md'), + ]; + + for (const proposeFile of proposeFiles) { + expect(await fileExists(proposeFile), proposeFile).toBe(true); + const content = await fs.readFile(proposeFile, 'utf-8'); + expect(content, proposeFile).toContain('**Planning boundary**'); + expect(content, proposeFile).toContain( + 'selected or triggered this workflow authorizes planning only' + ); + expect(content, proposeFile).toContain('ambiguity that would materially affect scope'); + expect(content, proposeFile).toContain( + 'ask the user before creating the change' + ); + expect(content, proposeFile).toContain( + 'Any implementation or apply instruction in that request does not carry forward' + ); + expect(content, proposeFile).toContain( + 'wait for a new user request to start the apply workflow' + ); + } + }); + it('should select all tools with --tools all option', async () => { const initCommand = new InitCommand({ tools: 'all', force: true }); diff --git a/test/core/templates/propose.test.ts b/test/core/templates/propose.test.ts index f8f8842e1d..429c6e89d2 100644 --- a/test/core/templates/propose.test.ts +++ b/test/core/templates/propose.test.ts @@ -9,10 +9,19 @@ import { getOpsxFfCommandTemplate, } from '../../../src/core/templates/skill-templates.js'; import { loadSchema } from '../../../src/core/artifact-graph/schema.js'; +import { CommandAdapterRegistry } from '../../../src/core/command-generation/registry.js'; +import { generateCommand } from '../../../src/core/command-generation/generator.js'; +import { + formatCommandInvocation, + getInvocationForAdapter, +} from '../../../src/core/command-generation/invocation.js'; +import { getCommandContents } from '../../../src/core/shared/skill-generation.js'; +const proposeSkillBody = getOpsxProposeSkillTemplate().instructions; +const proposeCommandBody = getOpsxProposeCommandTemplate().content; const proposeBodies: Array<[string, string]> = [ - ['propose skill', getOpsxProposeSkillTemplate().instructions], - ['propose command', getOpsxProposeCommandTemplate().content], + ['propose skill', proposeSkillBody], + ['propose command', proposeCommandBody], ]; // ff runs the byte-identical artifact loop, so it carries the identical guards. @@ -28,7 +37,7 @@ const defaultSchema = loadSchema(path.join(repoRoot, 'schemas', 'spec-driven', ' /** The opening list that tells the agent which artifacts propose will produce. */ function artifactPreamble(body: string): string { const start = body.indexOf("I'll create a change with"); - const end = body.indexOf('When ready to implement'); + const end = body.indexOf('When the user is ready to implement'); expect(start).toBeGreaterThanOrEqual(0); expect(end).toBeGreaterThan(start); return body.slice(start, end); @@ -51,6 +60,99 @@ describe('propose preamble', () => { }); }); +describe('propose implementation boundary', () => { + it('makes the planning-only boundary prominent (#232, #258, #262)', () => { + for (const [label, body] of proposeBodies) { + const boundary = body.indexOf('**Planning boundary**'); + const steps = body.indexOf('**Steps**'); + expect(boundary, `${label} is missing its planning boundary`).toBeGreaterThanOrEqual(0); + expect(boundary, `${label} boundary should appear before its steps`).toBeLessThan(steps); + expect(body, label).toContain( + 'The user request that selected or triggered this workflow authorizes planning only' + ); + expect(body, label).toContain('Do not edit project code'); + } + }); + + it('ends by requiring a separate apply workflow (#258, #262)', () => { + for (const [label, body] of proposeBodies) { + expect(body, label).toContain( + 'The request that invoked this workflow authorizes planning only' + ); + expect(body, label).toContain('Do NOT implement the change'); + expect(body, label).toContain('edit project code'); + expect(body, label).toContain( + 'Do not start implementation in the same response' + ); + expect(body, label).toContain( + 'Any implementation or apply instruction in that request does not carry forward' + ); + expect(body, label).toContain( + 'wait for a new user request to start the apply workflow' + ); + expect( + body.lastIndexOf('After presenting the artifacts, stop'), + `${label} should end with its stop guard` + ).toBeGreaterThan(body.indexOf('**Output**')); + } + }); + + it('asks before resolving ambiguity that could change user-visible outcomes (#258)', () => { + for (const [label, body] of proposeBodies) { + expect(body, label).toContain( + 'scope, externally observable behavior, compatibility, or acceptance criteria' + ); + expect(body, label).toContain('ask the user before creating the change'); + expect(body, label).toContain( + 'For minor details, make a reasonable assumption and record it in the planning artifacts' + ); + expect(body.indexOf('ask the user before creating the change'), label) + .toBeLessThan(body.indexOf('**Create the change directory**')); + } + }); + + it('hands command-only tools to apply instead of advertising direct coding (#258)', () => { + expect(proposeCommandBody).toContain('When you are ready, run `/opsx:apply`.'); + expect(proposeCommandBody).not.toContain('ask me to implement'); + expect(proposeCommandBody).not.toContain('ask me to apply this change'); + + expect(proposeSkillBody).toContain( + 'run `/opsx:apply` or ask me to apply this change' + ); + expect(proposeSkillBody).not.toContain('ask me to implement'); + }); + + it('preserves both boundaries through every command adapter', () => { + const propose = getCommandContents(['propose'])[0]; + expect(propose?.id).toBe('propose'); + + for (const adapter of CommandAdapterRegistry.getAll()) { + const generated = generateCommand(propose, adapter).fileContent; + const applyInvocation = formatCommandInvocation( + getInvocationForAdapter(adapter), + 'apply' + ); + expect(generated, adapter.toolId).toContain( + 'selected or triggered this workflow authorizes planning only' + ); + expect(generated, adapter.toolId).toContain('Do NOT implement the change'); + expect(generated, adapter.toolId).toContain( + 'Do not start implementation in the same response' + ); + expect(generated, adapter.toolId).toContain( + 'Any implementation or apply instruction in that request does not carry forward' + ); + expect(generated, adapter.toolId).toContain( + 'wait for a new user request to start the apply workflow' + ); + expect(generated, adapter.toolId).toContain( + `When you are ready, run \`${applyInvocation}\`.` + ); + expect(generated, adapter.toolId).not.toContain('ask me to implement'); + } + }); +}); + describe('artifact loop guards (propose and ff)', () => { // `status` is file-existence based (detectCompleted), so writing tasks.md before // specs flips tasks to done and satisfies a bare applyRequires stop condition diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 4c07f70278..7a17b6ed55 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -57,8 +57,8 @@ const EXPECTED_FUNCTION_HASHES: Record = { getOpsxOnboardCommandTemplate: 'e04e4ab6c2f25122e6840212b4c22708812c36ceff9ec529c2bb1d1d035429e3', getOpsxBulkArchiveCommandTemplate: 'fbb4de58ed00861badd93cde9bdd3d7c52f966158a18a660152060076ea9723e', getOpsxVerifyCommandTemplate: 'ce0ee05b7a6b332e29db2298b9d5a928a1932caf516e35fd88f163154ffd43f4', - getOpsxProposeSkillTemplate: '6d098be13fc130b592427323dcbe505e865e54d2070cf9f8b7157890fdbaf77a', - getOpsxProposeCommandTemplate: '53eb694ac6a2cb865500b41c1bae45fc71016e40129ce10f9056f1b6068972d8', + getOpsxProposeSkillTemplate: 'e175316cc654f78fea4195ee3f5173e544cc3bae35585e200833f26abbb09bd7', + getOpsxProposeCommandTemplate: '1085c01d9ce9ca576eab43887a6700007f30001978b624f7e004df7beb577028', getFeedbackSkillTemplate: 'd7d83c5f7fc2b92fe8f4588a5bf2d9cb315e4c73ec19bcd5ef28270906319a0d', getUpdateChangeSkillTemplate: 'f85fbfb3a175e949becbef08be0eccfab97de5e7ad45105e999d2900dfafbaba', getOpsxUpdateCommandTemplate: '461edf06e92c0da3dab4f11d91d59d44b48ed30a0881c1f34a714b1813435af6', @@ -75,7 +75,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-bulk-archive-change': 'c58e1d319a6587b52202434d5d769c94718aafc0f019276cef04cf8be473b6ce', 'openspec-verify-change': '7cd65897d126f7c948620c0672ca62418620dbcb82ee73d890f758fb666a4ff8', 'openspec-onboard': '80f39cf33a138aac8e508db25d7af2c9e9bd482f90e414770e806f966dd58c9c', - 'openspec-propose': '890f7c46e0ce7bbae5c8b214bd000d4c50c7d2f13aec1a55284c2fea77f13536', + 'openspec-propose': '37818ab54ffc8e60a51ec8cd9913eec8735645ea0c6c46a19e89de9b573dcf2c', 'openspec-update-change': '95bb533105e49aee06c9ea164b63092de77644cf8f94fa38d3ee3c11b0ccb893', };