feat(opsx): add /opsx:clarify command for interactive spec clarification#702
Open
robdefeo wants to merge 12 commits intoFission-AI:mainfrom
Open
feat(opsx): add /opsx:clarify command for interactive spec clarification#702robdefeo wants to merge 12 commits intoFission-AI:mainfrom
robdefeo wants to merge 12 commits intoFission-AI:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant Agent
participant SpecArtifacts as Spec Artifacts
participant Clarifications
User->>Agent: Invoke /opsx:clarify
Agent->>SpecArtifacts: Load all spec files (.md)
Agent->>Agent: Scan for ambiguities (10-category taxonomy)
Agent->>Agent: Generate prioritized question queue (max 5)
loop Interactive Q&A Loop
Agent->>User: Present question (MCQ or short-answer)
User->>Agent: Provide answer or defer
alt Answer Provided
Agent->>Agent: Validate response
Agent->>Clarifications: Store answer with category & timestamp
else Deferred
Agent->>Agent: Track deferred item
end
end
Agent->>SpecArtifacts: Apply clarifications to relevant sections
SpecArtifacts->>SpecArtifacts: Update requirements, add scenarios, glossary entries
Agent->>Agent: Generate coverage summary (resolved/deferred/outstanding)
Agent->>User: Report completion with coverage by category
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
I've been using this locally successfully and it's working as expected and helping to reduce ambiguity in specs |
Greptile OverviewGreptile SummaryThis PR adds a new Key Changes:
The implementation is thorough, well-documented, and follows OpenSpec patterns. The clarify command fills an important gap in the workflow by helping users identify and resolve specification ambiguities before implementation, reducing rework and improving implementation outcomes. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Agent as AI Agent
participant CLI as OpenSpec CLI
participant Specs as Spec Artifacts
User->>Agent: /opsx:clarify [change-name]
Agent->>CLI: openspec list --json
CLI-->>Agent: Available changes
opt Change selection needed
Agent->>User: AskUserQuestion (select change)
User-->>Agent: Selected change
end
Agent->>CLI: openspec status --change "name" --json
CLI-->>Agent: Artifact status (check specs exist)
alt No specs exist
Agent->>User: "Create specs first with /opsx:continue"
else Specs exist
Agent->>Specs: Read all spec files (specs/**/*.md)
Specs-->>Agent: Spec content
Agent->>Agent: Scan for ambiguities (10 categories)
Agent->>Agent: Prioritize top 5 questions
alt No ambiguities found
Agent->>User: "Specs are clear! No ambiguities detected."
else Ambiguities found
loop For each question (max 5)
Agent->>User: Present question with recommendations
User-->>Agent: Provide answer / skip / done
alt Answer provided
Agent->>Agent: Validate answer
Agent->>Specs: Write to ## Clarifications section
Agent->>Specs: Update relevant spec sections
Specs-->>Agent: Write confirmed
else User skips
Agent->>Agent: Mark as deferred
else User requests stop
Agent->>Agent: Exit Q&A loop
end
end
Agent->>User: Coverage summary (resolved/deferred/outstanding)
end
end
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@docs/commands.md`:
- Around line 346-351: Add a blank line between the preceding paragraph and the
"Arguments:" heading/table to satisfy markdownlint MD058; locate the
"Arguments:" heading and the pipe-table that follows (the table with
`change-name`) and insert one empty line above the table so the table is
separated from the preceding text.
- Around line 338-345: Add a language specifier to the fenced code blocks that
show the `/opsx:clarify [change-name]` example under the "### `/opsx:clarify`"
section: change the triple-backtick fences to ```text so the block containing
"/opsx:clarify [change-name]" (and the second identical example later in the
file) is marked as text to satisfy markdownlint MD040.
In `@openspec/changes/add-clarify-command/design.md`:
- Line 5: The design currently contradicts the docs by saying the clarification
workflow is agent-only while the PR/docs mention a /opsx:clarify command;
reconcile this by deciding and documenting one approach and implement
accordingly: either (A) make the clarification workflow an agent skill template
(e.g., ClarifySkill, clarifySkillTemplate) and update docs/PR text to remove
/opsx:clarify, or (B) implement /opsx:clarify as a thin CLI/command wrapper that
invokes the agent skill (e.g., handler that calls ClarifySkill.run or
Agent.invokeClarify) and update design.md to describe the wrapper role; ensure
all references (design.md, PR objectives, docs) consistently state which option
is chosen and link the command name /opsx:clarify to the agent skill identifier
used in code.
- Around line 66-68: Remove the stray double-quote suffix in the first bullet
and fix the spacing in the function name in Decision 3: change the malformed
`getClarify Change SkillTemplate()` to a single identifier matching the naming
pattern, e.g., `getClarifyChangeSkillTemplate()` so the function name exactly
matches the skill name and follows the openspec-<action>-<optional-variant>
convention.
In `@openspec/changes/add-clarify-command/specs/interactive-qa-protocol/spec.md`:
- Around line 70-73: Update the "Short-answer format validation" scenario to
explicitly state the maximum short-answer length (≤5 words) so implementations
can enforce it; modify the Scenario header or its THEN clause (the "Short-answer
format validation" scenario and its validation step) to read that the system
checks the response meets both minimum requirements and a maximum of 5 words (or
equivalent character/token limit), and ensure the wording is unambiguous so
validators (and any function/class that enforces this rule) can rely on a clear
≤5-words cap.
In
`@openspec/changes/add-clarify-command/specs/openspec-clarify-skill-template/spec.md`:
- Around line 162-165: The scenario "Clarifications section exists" currently
creates a new session timestamp per answer; change it to use one session header
per run by appending new Q/A entries under the existing Clarifications session
header and its single session timestamp instead of creating a new timestamped
header for every appended answer — update the scenario text and acceptance
criteria to say "THEN the new answer is appended under the existing
Clarifications session timestamp" and ensure references to "session timestamp"
and "Clarifications section" reflect single-session behavior.
In `@openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md`:
- Around line 88-91: The scenario titled "Scenario: File backup before write"
over-promises because the steps only verify existence/writability; update the
spec by either adding an explicit backup step (e.g., add a WHEN/THEN pair that
performs/validates creating a backup copy before modification) or rename the
scenario/header to accurately reflect behavior (e.g., "Scenario: Verify file
exists and is writable before write") and update any references; edit the header
"Scenario: File backup before write" and the WHEN/THEN block accordingly to keep
title and behavior aligned.
In `@src/core/templates/index.ts`:
- Line 19: Add the new clarify skill into the skill/command generation: import
getClarifyChangeSkillTemplate into the skill-generation module, add
getClarifyChangeSkillTemplate to the array returned by getSkillTemplates(),
create a corresponding getOpsxClarifyCommandTemplate function (mirroring the
pattern of existing command template functions) and add it to the array returned
by getCommandTemplates(); ensure the import names and array entries match the
existing naming conventions so getClarifyChangeSkillTemplate and
getOpsxClarifyCommandTemplate are both included in their respective template
lists.
In `@src/core/templates/skill-templates.ts`:
- Around line 3485-3753: The getClarifyChangeSkillTemplate() lacks its paired
command template; add a new function named getOpsxClarifyCommandTemplate() that
mirrors other getOpsx*CommandTemplate() implementations (same structure/return
type as peers, matching command name to the skill, wiring
metadata/description/license, and referencing the skill produced by
getClarifyChangeSkillTemplate() where appropriate) so the skill/command pair
pattern is restored; also change the metadata.version value in the
getClarifyChangeSkillTemplate() return from '2.0' to '1.0' to match existing
templates (or document intentional versioning if you intend to keep '2.0').
🧹 Nitpick comments (1)
docs/commands.md (1)
401-403: Example uses "Category: Security" which isn't one of the 10 taxonomy categories.The taxonomy defines "Non-Functional Quality Attributes" (which includes security), not a standalone "Security" category. Consider updating to
*Category: Non-Functional*for consistency with the taxonomy table above at Lines 359-372.
openspec/changes/add-clarify-command/specs/interactive-qa-protocol/spec.md
Show resolved
Hide resolved
openspec/changes/add-clarify-command/specs/openspec-clarify-skill-template/spec.md
Show resolved
Hide resolved
openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md
Outdated
Show resolved
Hide resolved
Add getOpsxClarifyCommandTemplate function that generates the slash command template for the clarify workflow. This enables AI agents to use /opsx:clarify to identify and resolve spec ambiguities. Changes: - Add getOpsxClarifyCommandTemplate() to skill-templates.ts - Export function in skill-generation.ts - Add 'clarify' command to getCommandTemplates() array - Update tests to expect 11 templates (was 10) - Add 'clarify' and 'openspec-clarify-change' to expected lists The command template provides: - Interactive Q&A workflow with up to 5 questions - 10-category ambiguity taxonomy - AI-generated recommendations for each question - Incremental spec updates with timestamped sessions - Coverage reporting All tests pass (1217 tests).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/commands.md`:
- Line 376: The fenced code block that contains the line "You: /opsx:clarify" is
missing a language specifier and triggers MD040; update the opening fence for
that code block to include a language (e.g., add "text" after the triple
backticks) so the block becomes a fenced code block with a language specifier
and resolves the lint error.
- Line 403: The example shows "Category: Security" which doesn't match the
defined taxonomy; update the example text "Question 2 of 5 - *Category:
Security*" to use one of the actual categories from the taxonomy (e.g., replace
"Security" with "Non-Functional") so the example aligns with the table; modify
the string in docs/commands.md where that example appears accordingly.
In `@openspec/changes/add-clarify-command/design.md`:
- Around line 63-71: The document contains a contradiction: the chosen skill
name is "openspec-clarify" (Decision 3) while the Alternatives list also rejects
"openspec-clarify" due to a supposed v1 conflict; remove or reconcile that
leftover. Edit the Alternatives section to either delete the rejected
alternative entry "openspec-clarify" or replace it with a clarified note
differentiating the rejected variant (e.g., a conflicting v1 name) and ensure
the final decision and the function name getClarifyChangeSkillTemplate()
consistently reference "openspec-clarify".
In `@openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md`:
- Around line 118-120: The scenario title "Write failure halts Q&A" contradicts
the THEN clause which says the loop "pauses and reports the error before
continuing"; update the spec so the title matches the behavior: either change
the title to "Write failure pauses Q&A" (if the loop resumes after reporting) or
change the THEN clause to state the loop terminates (if it should halt). Edit
the scenario header and/or the THEN line in the spec (where "Write failure halts
Q&A" and "- **THEN** the Q&A loop pauses and reports the error before
continuing" appear) so they consistently describe the same outcome.
🧹 Nitpick comments (1)
src/core/shared/skill-generation.ts (1)
27-31: Import placement ofgetClarifyChangeSkillTemplateis inconsistent with grouping.The other skill template imports (
get*SkillTemplate) are grouped together at lines 8–17, butgetClarifyChangeSkillTemplateis placed at line 30 after the command template imports. This is a minor readability nit — consider moving it next to the other skill template imports for consistency.✏️ Suggested reorder
import { getExploreSkillTemplate, getNewChangeSkillTemplate, getContinueChangeSkillTemplate, getApplyChangeSkillTemplate, getFfChangeSkillTemplate, getSyncSpecsSkillTemplate, getArchiveChangeSkillTemplate, getBulkArchiveChangeSkillTemplate, getVerifyChangeSkillTemplate, + getClarifyChangeSkillTemplate, getOnboardSkillTemplate, getOpsxExploreCommandTemplate, ... getOpsxVerifyCommandTemplate, getOpsxClarifyCommandTemplate, getOpsxOnboardCommandTemplate, type SkillTemplate, - getClarifyChangeSkillTemplate, } from '../templates/skill-templates.js';
openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@openspec/changes/add-clarify-command/design.md`:
- Around line 10-11: The bullets "Provide detailed step-by-step instructions in
markdown format" and "Instructions reference OpenSpec CLI commands for status
checks and file operations" should use the proper noun "Markdown" with a capital
M; update those strings (and the other occurrences noted at the same file around
lines 40 and 100) to "Markdown" for consistent capitalization across
openspec/changes/add-clarify-command/design.md.
- Around line 101-109: Update the "Short-answer" specification by removing the
explicit "≤5 word answers" cap in the design text and either delete that phrase
or replace it with "concise" so it aligns with the interactive Q&A protocol and
the `/opsx:clarify` implementation; ensure the rationale section no longer
references a 5-word limit and add a note that short-answer responses follow the
Q&A protocol (no hard word limit) so the "Short-answer" bullet and rationale
stay consistent with the `/opsx:clarify` behavior.
In `@openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md`:
- Around line 40-87: The two requirements conflict: "Requirement: Spec section
updates from clarifications" (where requirement text is updated when clarified)
and "Requirement: File write safety" (which currently states "all original
requirements, scenarios, and sections remain intact")—update the File write
safety wording to remove the absolute preservation claim and make it explicit
that only unrelated content must remain unchanged; e.g., change the preservation
clause to state that clarifications may modify targeted requirement text and
related scenarios but must not alter unrelated requirements or metadata, and add
a brief note in "Scenario: Existing content unchanged" clarifying that
intactness applies only to unaffected sections.
- Around line 92-105: Update the requirement text to use the proper noun
capitalization "Markdown" in the header and any occurrences in the paragraph
titled "Requirement: Markdown formatting preservation" and ensure the scenarios
(e.g., "Header hierarchy preserved", "Session headers use correct level",
"Question formatting uses bold") retain the same wording but reference
"Markdown" consistently; edit the phrase "The system SHALL maintain proper
markdown formatting when integrating clarifications." to "The system SHALL
maintain proper Markdown formatting when integrating clarifications." to fix the
capitalization.
openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md
Show resolved
Hide resolved
openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/commands.md`:
- Line 445: Remove the misleading "Short-answer questions should be concise (≤5
words when possible)" suggestion from the docs entry that references
short-answer guidance, and update any nearby text referencing short-answer
length to reflect that the /opsx:clarify command does not enforce a maximum word
limit; specifically edit the line containing "Short-answer questions should be
concise (≤5 words when possible)" in docs/commands.md and ensure the
short-answer guidance aligns with the /opsx:clarify implementation and the skill
template (remove the ≤5-word constraint and keep guidance that short answers
should be concise without specifying a hard word limit).
- Around line 426-429: The example under the "By Category" section still lists
the category label "Security: 2 resolved" which is inconsistent with the
taxonomy that uses "Non-Functional"; update the example output string "Security:
2 resolved" to "Non-Functional: 2 resolved" (search for the literal "Security: 2
resolved" or the "By Category" block and replace that entry) so the example
aligns with the taxonomy terminology used elsewhere.
In
`@openspec/changes/add-clarify-command/specs/openspec-clarify-skill-template/spec.md`:
- Around line 10-12: The spec requires a generatedBy metadata field but
getClarifyChangeSkillTemplate() currently returns only author and version;
update the implementation in getClarifyChangeSkillTemplate (in
skill-templates.ts) to include metadata.generatedBy (e.g., 'openspec' or an
appropriate value) alongside author and version so the returned template matches
the spec, or alternatively update the spec to remove generatedBy—preferably add
metadata.generatedBy to the function's returned object to preserve the spec's
contract.
In `@openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md`:
- Around line 40-41: Update the requirement text under "Requirement: Spec
section updates from clarifications" by replacing the awkward sentence "The
system SHALL apply clarification answers to relevant spec in the
`Clarifications` sections to resolve ambiguities." with the clearer phrasing
"The system SHALL apply clarification answers to relevant spec sections and
record them in the `Clarifications` section to resolve ambiguities." so it
explicitly states both where the spec is updated and where the record of the
clarification is kept.
In `@src/core/templates/skill-templates.ts`:
- Around line 3856-3866: Update the short-answer example in the template block
labeled "B. **Short-answer (for open-ended)**" by removing the phrase "(≤5 words
if possible)" from the prompt line that currently reads "Please provide answer
(≤5 words if possible):" so it instead asks for an unrestricted short answer;
ensure this change aligns the example with the existing short-answer handling
used by the /opsx:clarify flow and the skill template elsewhere.
openspec/changes/add-clarify-command/specs/openspec-clarify-skill-template/spec.md
Outdated
Show resolved
Hide resolved
openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md
Outdated
Show resolved
Hide resolved
… spec integration rules
Contributor
|
@robdefeo, I have an idea on something similar here. I'll revisit this when some of the skills installation thing is up. Current problem is users have too many skills so we want to make some of them optional to install. |
Author
@TabishB thanks for the feedback. I see the dilemma, tools start off simple then are more people use it they want extra functionality which makes it more useful but also more complex. Open to anything that helps this. Is the skills instructions this PR npx skills add https://github.com/softaworks/agent-toolkit --skill mermaid-diagrams or something else? |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/core/templates/workflows/clarify-change.ts (1)
9-278: Significant content duplication between the two templates.
getClarifyChangeSkillTemplate()andgetOpsxClarifyCommandTemplate()share ~80% identical text: the full 10-category taxonomy, workflow steps, integration guidelines, guardrails, and example session are duplicated verbatim. Any future edit (e.g., adding an 11th category or tweaking a guideline) must be replicated in both places.Consider extracting the shared content (taxonomy, guidelines, guardrails, example) into shared constants or a builder function, and composing each template from those parts.
♻️ Sketch of a possible approach
// Shared content fragments const AMBIGUITY_TAXONOMY = `...10 categories...`; const SPEC_INTEGRATION_GUIDELINES = `...guidelines...`; const GUARDRAILS = `...guardrails...`; const EXAMPLE_SESSION = `...example...`; export function getClarifyChangeSkillTemplate(): SkillTemplate { return { name: 'openspec-clarify', description: '...', instructions: `...skill-specific preamble... ${AMBIGUITY_TAXONOMY} ...skill-specific Q&A format... ${SPEC_INTEGRATION_GUIDELINES} ${GUARDRAILS} ${EXAMPLE_SESSION}`, license: 'MIT', compatibility: 'Requires openspec CLI.', metadata: { author: 'openspec', version: '1.0' }, }; } export function getOpsxClarifyCommandTemplate(): CommandTemplate { return { name: 'OPSX: Clarify', description: '...', category: 'Workflow', tags: ['workflow', 'clarify', 'experimental'], content: `...command-specific preamble... ${AMBIGUITY_TAXONOMY} ...command-specific Q&A format... ${SPEC_INTEGRATION_GUIDELINES} ${GUARDRAILS} ${EXAMPLE_SESSION}`, }; }Also applies to: 280-517
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/templates/workflows/clarify-change.ts` around lines 9 - 278, The two templates getClarifyChangeSkillTemplate() and getOpsxClarifyCommandTemplate() duplicate ~80% of their instructions (taxonomy, guidelines, guardrails, example session); refactor by extracting the shared text into named constants (e.g., AMBIGUITY_TAXONOMY, SPEC_INTEGRATION_GUIDELINES, GUARDRAILS, EXAMPLE_SESSION) or a helper builder function (e.g., buildClarifyInstructions({preamble, mode})) and then compose each template's instructions from those shared fragments while keeping only the template-specific preamble/formatting inline; update both functions to use the new constants/builder so future edits change in one place.openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md (1)
118-127: Path handling scenarios over-specify platform details.Lines 123 and 127 both prescribe
path.join(), which is the actual requirement. Mentioning "backslashes" (Windows) and "forward slashes" (Unix) as separate THEN clauses is redundant sincepath.join()already handles this. Consider simplifying both scenarios into a single platform-agnostic scenario, or drop the slash-direction detail and just assertpath.join()usage.✏️ Suggested simplification
-#### Scenario: Spec file paths resolved correctly on Windows -- **WHEN** clarifications are written to specs on Windows -- **THEN** file paths use backslashes and path.join() for correctness - -#### Scenario: Spec file paths resolved correctly on Unix -- **WHEN** clarifications are written to specs on macOS or Linux -- **THEN** file paths use forward slashes and path.join() for correctness +#### Scenario: Spec file paths resolved correctly across platforms +- **WHEN** clarifications are written to specs on any supported OS +- **THEN** file paths are constructed using path.join() for platform-independent correctness🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md` around lines 118 - 127, The two scenarios under "Requirement: Path handling in spec updates" over-specify platform slash directions; consolidate them into a single platform-agnostic scenario that asserts use of path.join() for file operations (refer to the "Path handling in spec updates" heading and the two scenarios currently titled "Spec file paths resolved correctly on Windows" and "Spec file paths resolved correctly on Unix"), removing the explicit "backslashes" and "forward slashes" THEN clauses and keeping a single WHEN/THEN that states clarifications are written to specs and file paths are constructed using path.join() to ensure platform-independent handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/add-clarify-command/proposal.md`:
- Around line 32-33: Update the proposal's "New files" list to accurately
reflect the added file by mentioning clarify-change.ts and the two exported
functions (getClarifyChangeSkillTemplate and getOpsxClarifyCommandTemplate);
edit the proposal.md section that currently says "New files: None" so it lists
clarify-change.ts as a new file and briefly note the added functions to match
the implementation.
- Around line 56-68: The template metadata version in the proposal is "2.0" but
the implementation in src/core/templates/workflows/clarify-change.ts returns
version '1.0'; update the implementation to match the proposal by changing the
returned template metadata version string in the clarify-change.ts template (the
object returned by the workflow/template function that currently contains
version: '1.0') to "2.0" so both proposal.md and the clarify-change.ts template
use the same version.
In
`@openspec/changes/add-clarify-command/specs/openspec-clarify-skill-template/spec.md`:
- Around line 192-194: The scenario titled "Additional ambiguities discovered"
overstates runtime behavior—update the scenario header and WHEN clause to
reflect the actual implementation (single upfront scan) by replacing "WHEN spec
updates reveal new ambiguities" with a condition like "WHEN the question quota
is exhausted and unaddressed ambiguities remain" (or similar wording), keep the
THEN clause that recommends re-running /opsx:clarify, and ensure the scenario
text (the "#### Scenario: Additional ambiguities discovered" header, the WHEN
line, and the THEN line) consistently describes that ambiguities were identified
in the initial scan rather than discovered during runtime.
---
Nitpick comments:
In `@openspec/changes/add-clarify-command/specs/spec-integration-rules/spec.md`:
- Around line 118-127: The two scenarios under "Requirement: Path handling in
spec updates" over-specify platform slash directions; consolidate them into a
single platform-agnostic scenario that asserts use of path.join() for file
operations (refer to the "Path handling in spec updates" heading and the two
scenarios currently titled "Spec file paths resolved correctly on Windows" and
"Spec file paths resolved correctly on Unix"), removing the explicit
"backslashes" and "forward slashes" THEN clauses and keeping a single WHEN/THEN
that states clarifications are written to specs and file paths are constructed
using path.join() to ensure platform-independent handling.
In `@src/core/templates/workflows/clarify-change.ts`:
- Around line 9-278: The two templates getClarifyChangeSkillTemplate() and
getOpsxClarifyCommandTemplate() duplicate ~80% of their instructions (taxonomy,
guidelines, guardrails, example session); refactor by extracting the shared text
into named constants (e.g., AMBIGUITY_TAXONOMY, SPEC_INTEGRATION_GUIDELINES,
GUARDRAILS, EXAMPLE_SESSION) or a helper builder function (e.g.,
buildClarifyInstructions({preamble, mode})) and then compose each template's
instructions from those shared fragments while keeping only the
template-specific preamble/formatting inline; update both functions to use the
new constants/builder so future edits change in one place.
Comment on lines
+32
to
+33
| **New files:** | ||
| - None - skill template is added inline to existing `skill-templates.ts` |
Contributor
There was a problem hiding this comment.
Proposal claims no new files, but clarify-change.ts is a new file.
The "New files: None" statement is inaccurate. The implementation adds src/core/templates/workflows/clarify-change.ts as a new file containing getClarifyChangeSkillTemplate() and getOpsxClarifyCommandTemplate(). Consider updating this section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/add-clarify-command/proposal.md` around lines 32 - 33,
Update the proposal's "New files" list to accurately reflect the added file by
mentioning clarify-change.ts and the two exported functions
(getClarifyChangeSkillTemplate and getOpsxClarifyCommandTemplate); edit the
proposal.md section that currently says "New files: None" so it lists
clarify-change.ts as a new file and briefly note the added functions to match
the implementation.
Comment on lines
+56
to
+68
| ```markdown | ||
| --- | ||
| name: openspec-clarify | ||
| description: Clarify underspecified areas in an OpenSpec change by asking up to 5 targeted questions and writing answers back into spec artifacts. | ||
| license: MIT | ||
| compatibility: Requires openspec CLI. | ||
| metadata: | ||
| author: openspec | ||
| version: "2.0" | ||
| --- | ||
|
|
||
| [Full skill template content with 8 steps: Select change, Check status, Load specs, Scan for ambiguity using taxonomy, Generate question queue, Interactive Q&A loop with recommendations, Integrate answers into specs, Validate and report] | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the proposal file
find . -name "proposal.md" -path "*clarify*" 2>/dev/null
# Then search for the implementation file with clarify in the name
find . -name "*clarify*.ts" 2>/dev/null | head -20Repository: Fission-AI/OpenSpec
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Search for version declarations in TypeScript files related to clarify
rg -n "version\s*[:=]" --type=ts -C3 | grep -i clarify -A5 -B5Repository: Fission-AI/OpenSpec
Length of output: 1274
🏁 Script executed:
#!/bin/bash
# Read the proposal.md file, specifically lines 56-68
sed -n '56,68p' ./openspec/changes/add-clarify-command/proposal.mdRepository: Fission-AI/OpenSpec
Length of output: 594
Template metadata version mismatch: proposal declares "2.0", implementation uses "1.0".
The proposal file specifies version: "2.0" (line 64), but src/core/templates/workflows/clarify-change.ts (line 276) returns version: '1.0'. Align the versions between the proposal and implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/add-clarify-command/proposal.md` around lines 56 - 68, The
template metadata version in the proposal is "2.0" but the implementation in
src/core/templates/workflows/clarify-change.ts returns version '1.0'; update the
implementation to match the proposal by changing the returned template metadata
version string in the clarify-change.ts template (the object returned by the
workflow/template function that currently contains version: '1.0') to "2.0" so
both proposal.md and the clarify-change.ts template use the same version.
Comment on lines
+192
to
+194
| #### Scenario: Additional ambiguities discovered | ||
| - **WHEN** spec updates reveal new ambiguities | ||
| - **THEN** the report notes outstanding items and recommends another clarification pass |
Contributor
There was a problem hiding this comment.
"Additional ambiguities discovered" scenario may overstate current implementation capabilities.
This scenario implies the system detects new ambiguities introduced by spec updates during the session. The actual implementation performs a single scan upfront and does not re-scan after each answer. The THEN clause ("notes outstanding items and recommends another clarification pass") aligns with what the implementation does (suggest re-running /opsx:clarify), but the WHEN clause ("spec updates reveal new ambiguities") implies runtime detection that doesn't occur. Consider rewording to match reality — e.g., "WHEN the question quota is exhausted and unaddressed ambiguities remain."
✏️ Suggested rewording
-#### Scenario: Additional ambiguities discovered
-- **WHEN** spec updates reveal new ambiguities
-- **THEN** the report notes outstanding items and recommends another clarification pass
+#### Scenario: Unaddressed ambiguities remain
+- **WHEN** the question quota is reached and unscanned or unaddressed ambiguities remain
+- **THEN** the report notes outstanding items and recommends another clarification pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/add-clarify-command/specs/openspec-clarify-skill-template/spec.md`
around lines 192 - 194, The scenario titled "Additional ambiguities discovered"
overstates runtime behavior—update the scenario header and WHEN clause to
reflect the actual implementation (single upfront scan) by replacing "WHEN spec
updates reveal new ambiguities" with a condition like "WHEN the question quota
is exhausted and unaddressed ambiguities remain" (or similar wording), keep the
THEN clause that recommends re-running /opsx:clarify, and ensure the scenario
text (the "#### Scenario: Additional ambiguities discovered" header, the WHEN
line, and the THEN line) consistently describes that ambiguities were identified
in the initial scan rather than discovered during runtime.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add new /opsx:clarify command that enables interactive resolution of
ambiguities in spec artifacts through a structured Q&A workflow.
Key additions:
non-functional requirements, integration, edge cases, constraints,
terminology, completion signals, misc/placeholders)
Documentation updates:
The clarify command helps users produce complete, unambiguous specs by
identifying gaps and capturing answers directly in spec artifacts, leading
to better implementation outcomes.
Summary by CodeRabbit
New Features
/opsx:clarifycommand for resolving specification ambiguities through interactive Q&A sessionsDocumentation
resolves #673