feat: add /opsx:refine command with scratchpad-based artifact review#893
Open
shohu wants to merge 3 commits intoFission-AI:mainfrom
Open
feat: add /opsx:refine command with scratchpad-based artifact review#893shohu wants to merge 3 commits intoFission-AI:mainfrom
shohu wants to merge 3 commits intoFission-AI:mainfrom
Conversation
… review Add a new workflow command that reviews change artifacts for cross-artifact consistency, best-practice alignment, and implementation readiness. Issues are tracked in a per-change scratchpad file (scratchpad.md) that persists across sessions and gets committed with artifact updates for convergence traceability. Inspired by @demianmnave's nhc-opsx-refine command (Fission-AI#783).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "User (CLI)"
participant OPSX as "opsx handler"
participant Skill as "Refine Skill"
participant Store as "Artifact Store"
participant User as "User (approve/modify)"
participant VCS as "VCS / Commit"
CLI->>OPSX: run `/opsx:refine [--change "<name>"]`
OPSX->>Skill: invoke refine workflow
Skill->>Store: `openspec list --json` / `openspec status --change "<name>" --json`
Store-->>Skill: return artifacts (proposal/specs/design/tasks, existing specs)
Skill->>Skill: create prioritized scratchpad.md (P0/P1/P2)
Skill->>User: present next issue + proposed edit
User-->>Skill: approve / modify / reject
Skill->>Store: apply approved artifact updates + add scratchpad.md
Skill->>Skill: run `openspec validate --change "<name>" --strict`
Skill->>VCS: commit scratchpad + artifact changes together
VCS-->>Skill: commit result
Skill->>CLI: final status (resolved / remaining counts)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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)
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 |
![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: 4
🧹 Nitpick comments (2)
test/core/shared/skill-generation.test.ts (1)
13-13: Strengthen these count updates with explicitrefinepresence checks.At Line 13, Line 93, and Line 147, only the totals changed. Please also assert
openspec-refine-change/refinein the corresponding expected-ID tests so regressions aren’t masked by a wrong extra entry. Also consider updating the test titles that still say “11”.Also applies to: 93-93, 147-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/shared/skill-generation.test.ts` at line 13, The length-only assertions for templates (e.g., expect(templates).toHaveLength(12) in the test/core/shared/skill-generation.test.ts tests at the three spots) are too weak; add explicit checks that the refine-related template is present by asserting an item with id 'openspec-refine-change' (or a template whose id/name includes 'refine') exists in the templates array alongside the length check, and update the test titles that still say "11" to the new count so the intent matches the expectation.src/core/templates/workflows/refine-change.ts (1)
152-155: Avoid brittle instruction mutation via literal.replace(...).The replacement depends on an exact sentence match; small wording edits to
refineInstructionscan silently break command-specific input text. Prefer composing command instructions from explicit sections/constants instead of relying on string replacement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/templates/workflows/refine-change.ts` around lines 152 - 155, The current assignment mutates refineInstructions via a brittle string.replace call; instead build the instruction text from explicit pieces: extract the base instructions (refineInstructions) and a separate constant or small string for the `/opsx:refine` change-name sentence (e.g., changeNameSection = 'The argument after `/opsx:refine` is the change name (e.g., `/opsx:refine add-auth`).'), then set content by concatenating or template-literaling those parts (for example: content: `${refineInstructions}\n\n${changeNameSection}`) rather than using refineInstructions.replace(...); update code in refine-change.ts to remove the replace usage and reference the new changeNameSection constant when composing content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/commands.md`:
- Around line 328-332: The Markdown table under the "**Arguments:**" heading
(the table that documents the `change-name` argument) needs blank lines before
and after it to satisfy markdownlint MD058; edit the section containing the
"**Arguments:**" heading and ensure there is an empty line between the heading
and the table and another empty line after the table so the table is separated
from surrounding text.
In `@src/core/templates/workflows/refine-change.ts`:
- Around line 62-82: The refine-change workflow never runs strict validation
before committing; modify the refine loop (the section that presents fixes,
applies them, updates the scratchpad, and performs commits) to run "openspec
validate --strict" after applying a proposed fix and before committing; if
validation fails, mark the current issue status as "Needs refinement", keep the
updated artifacts uncommitted, prompt the user for direction, update the
scratchpad state accordingly, and only proceed to commit the scratchpad and
artifacts when the strict validation passes and the user approves.
- Around line 36-60: Update the "Cross-artifact consistency checks" step (step
4) in the refine-change workflow to include an explicit duplication/reuse review
against the existing codebase and shared specs/components: add checks that
compare current change artifacts (Proposal, Specs, Design, Tasks) against
existing specs/components to detect potential reimplementations, flag reuse
opportunities, and record a recommended action (reuse/extend/duplicate) and risk
level in the scratchpad; ensure this new check is implemented alongside the
existing Proposal→Specs, Specs→Design, Design→Tasks, and Specs→Tasks comparisons
so findings are appended with priority (P0/P1/P2) and a reuse/duplication tag.
- Line 97: Replace the over-escaped backticks in the template examples by
removing the stray backslashes: locate the string "Tracks openspec-refine issues
and working decisions for the \\\`<change-name>\\\` change." and change it to
use a single escaped backtick sequence "Tracks openspec-refine issues and
working decisions for the \`<change-name>\` change.", and likewise update the
list item " - \\\`openspec/changes/<change-name>/...\\\`" to " -
\`openspec/changes/<change-name>/...\`" so markdown inline code renders
correctly.
---
Nitpick comments:
In `@src/core/templates/workflows/refine-change.ts`:
- Around line 152-155: The current assignment mutates refineInstructions via a
brittle string.replace call; instead build the instruction text from explicit
pieces: extract the base instructions (refineInstructions) and a separate
constant or small string for the `/opsx:refine` change-name sentence (e.g.,
changeNameSection = 'The argument after `/opsx:refine` is the change name (e.g.,
`/opsx:refine add-auth`).'), then set content by concatenating or
template-literaling those parts (for example: content:
`${refineInstructions}\n\n${changeNameSection}`) rather than using
refineInstructions.replace(...); update code in refine-change.ts to remove the
replace usage and reference the new changeNameSection constant when composing
content.
In `@test/core/shared/skill-generation.test.ts`:
- Line 13: The length-only assertions for templates (e.g.,
expect(templates).toHaveLength(12) in the
test/core/shared/skill-generation.test.ts tests at the three spots) are too
weak; add explicit checks that the refine-related template is present by
asserting an item with id 'openspec-refine-change' (or a template whose id/name
includes 'refine') exists in the templates array alongside the length check, and
update the test titles that still say "11" to the new count so the intent
matches the expectation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 70e2c0a9-6c63-4968-bd83-2f10937a9313
📒 Files selected for processing (6)
docs/commands.mdsrc/core/shared/skill-generation.tssrc/core/templates/skill-templates.tssrc/core/templates/workflows/refine-change.tstest/core/shared/skill-generation.test.tstest/core/templates/skill-templates-parity.test.ts
Comment on lines
+328
to
+332
| **Arguments:** | ||
| | Argument | Required | Description | | ||
| |----------|----------|-------------| | ||
| | `change-name` | No | Which change to refine (prompted if not provided) | | ||
|
|
Contributor
There was a problem hiding this comment.
Fix markdownlint MD058 in the refine arguments table.
Add blank lines around the table under **Arguments:** (Line 329 area) to satisfy lint.
✏️ Proposed docs fix
**Arguments:**
+
| Argument | Required | Description |
|----------|----------|-------------|
| `change-name` | No | Which change to refine (prompted if not provided) |
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Arguments:** | |
| | Argument | Required | Description | | |
| |----------|----------|-------------| | |
| | `change-name` | No | Which change to refine (prompted if not provided) | | |
| **Arguments:** | |
| | Argument | Required | Description | | |
| |----------|----------|-------------| | |
| | `change-name` | No | Which change to refine (prompted if not provided) | | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 329-329: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/commands.md` around lines 328 - 332, The Markdown table under the
"**Arguments:**" heading (the table that documents the `change-name` argument)
needs blank lines before and after it to satisfy markdownlint MD058; edit the
section containing the "**Arguments:**" heading and ensure there is an empty
line between the heading and the table and another empty line after the table so
the table is separated from surrounding text.
Comment on lines
+36
to
+60
| 4. **Cross-artifact consistency checks** | ||
|
|
||
| Compare artifacts against each other for contradictions, gaps, and ambiguities: | ||
|
|
||
| - **Proposal → Specs**: Does every goal in the proposal have corresponding requirements in specs? | ||
| - **Specs → Design**: Does the design address all requirements and scenarios? | ||
| - **Design → Tasks**: Do tasks cover all design decisions? Are implementation steps aligned? | ||
| - **Specs → Tasks**: Are all requirements traceable to at least one task? | ||
|
|
||
| Record each issue found in the scratchpad with priority: | ||
| - **P0**: Contradiction between artifacts (e.g., spec says X, design says Y) | ||
| - **P1**: Gap in coverage (e.g., requirement with no corresponding task) | ||
| - **P2**: Ambiguity or unclear intent (e.g., vague acceptance criteria) | ||
|
|
||
| 5. **Best-practice validation** | ||
|
|
||
| Check artifacts against common quality standards: | ||
|
|
||
| - Tasks have clear acceptance criteria | ||
| - Specs define measurable scenarios (Given/When/Then or equivalent) | ||
| - Design decisions include rationale | ||
| - No circular or missing dependencies in task ordering | ||
| - Edge cases and error states are addressed | ||
|
|
||
| Add findings to the scratchpad. |
Contributor
There was a problem hiding this comment.
Add an explicit duplication/reuse review against existing specs/components.
Cross-checks currently stay within change artifacts. The workflow should also inspect existing specs/shared components to flag reimplementation risk and suggest reuse vs duplication.
♻️ Proposed check to add
5. **Best-practice validation**
Check artifacts against common quality standards:
@@
- Edge cases and error states are addressed
+ - Existing `openspec/specs/` and shared components are reviewed to detect duplication
+ - Prefer reuse/refactor recommendations over reimplementation where feasible📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 4. **Cross-artifact consistency checks** | |
| Compare artifacts against each other for contradictions, gaps, and ambiguities: | |
| - **Proposal → Specs**: Does every goal in the proposal have corresponding requirements in specs? | |
| - **Specs → Design**: Does the design address all requirements and scenarios? | |
| - **Design → Tasks**: Do tasks cover all design decisions? Are implementation steps aligned? | |
| - **Specs → Tasks**: Are all requirements traceable to at least one task? | |
| Record each issue found in the scratchpad with priority: | |
| - **P0**: Contradiction between artifacts (e.g., spec says X, design says Y) | |
| - **P1**: Gap in coverage (e.g., requirement with no corresponding task) | |
| - **P2**: Ambiguity or unclear intent (e.g., vague acceptance criteria) | |
| 5. **Best-practice validation** | |
| Check artifacts against common quality standards: | |
| - Tasks have clear acceptance criteria | |
| - Specs define measurable scenarios (Given/When/Then or equivalent) | |
| - Design decisions include rationale | |
| - No circular or missing dependencies in task ordering | |
| - Edge cases and error states are addressed | |
| Add findings to the scratchpad. | |
| 4. **Cross-artifact consistency checks** | |
| Compare artifacts against each other for contradictions, gaps, and ambiguities: | |
| - **Proposal → Specs**: Does every goal in the proposal have corresponding requirements in specs? | |
| - **Specs → Design**: Does the design address all requirements and scenarios? | |
| - **Design → Tasks**: Do tasks cover all design decisions? Are implementation steps aligned? | |
| - **Specs → Tasks**: Are all requirements traceable to at least one task? | |
| Record each issue found in the scratchpad with priority: | |
| - **P0**: Contradiction between artifacts (e.g., spec says X, design says Y) | |
| - **P1**: Gap in coverage (e.g., requirement with no corresponding task) | |
| - **P2**: Ambiguity or unclear intent (e.g., vague acceptance criteria) | |
| 5. **Best-practice validation** | |
| Check artifacts against common quality standards: | |
| - Tasks have clear acceptance criteria | |
| - Specs define measurable scenarios (Given/When/Then or equivalent) | |
| - Design decisions include rationale | |
| - No circular or missing dependencies in task ordering | |
| - Edge cases and error states are addressed | |
| - Existing `openspec/specs/` and shared components are reviewed to detect duplication | |
| - Prefer reuse/refactor recommendations over reimplementation where feasible | |
| Add findings to the scratchpad. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/templates/workflows/refine-change.ts` around lines 36 - 60, Update
the "Cross-artifact consistency checks" step (step 4) in the refine-change
workflow to include an explicit duplication/reuse review against the existing
codebase and shared specs/components: add checks that compare current change
artifacts (Proposal, Specs, Design, Tasks) against existing specs/components to
detect potential reimplementations, flag reuse opportunities, and record a
recommended action (reuse/extend/duplicate) and risk level in the scratchpad;
ensure this new check is implemented alongside the existing Proposal→Specs,
Specs→Design, Design→Tasks, and Specs→Tasks comparisons so findings are appended
with priority (P0/P1/P2) and a reuse/duplication tag.
- Replace brittle .replace() with buildInstructions() + explicit constants - Fix over-escaped backticks in scratchpad template (\\\` → \`) - Add blank line before markdown table in commands.md (MD058) - Update test titles from "11" to "12" - Add explicit refine presence checks in skill/command tests
Collaborator
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Review: /opsx:refine — Cross-artifact Quality Review
Thanks for building this, @shohu! This is a natural evolution from #783 and fits cleanly into the SDD workflow philosophy — a quality gate between planning and implementation is exactly what users need (especially given the "LLMs skip to implementation" pattern we keep seeing in #258/#869/#875).
The architecture is clean: new workflow module, proper skill/command template split, buildInstructions DRY pattern, and the scratchpad-per-change scoping is the right design (no stale context across sessions). The commit-per-issue convergence traceability is a nice touch.
However, there are a few issues that should be addressed before merge:
🔴 Must Fix: refine not registered in ALL_WORKFLOWS (profiles.ts)
The workflow ID 'refine' is registered in skill-generation.ts (line 71, 98) but not added to ALL_WORKFLOWS in src/core/profiles.ts. This means:
- Custom profile users cannot select
refine— it's not in theWorkflowIdtype union - Core profile users won't get it (expected — it's an expanded workflow)
- Profile sync drift detection (
profile-sync-drift.ts) won't track it - The workflow only installs when
workflowFilterisundefined(no profile), which isn't the normal path
Fix:
// src/core/profiles.ts
export const ALL_WORKFLOWS = [
'propose',
'explore',
'new',
'continue',
'apply',
'ff',
'sync',
'archive',
'bulk-archive',
'verify',
'onboard',
+ 'refine',
] as const;🟠 Should Fix: AskUserQuestion tool reference (Cursor-specific)
Line 29 of the template uses Use the **AskUserQuestion tool** to let the user select — this is a Cursor-specific API that doesn't exist in Codex, Claude Code, Kilo Code, Pi, or most other agents. It fails silently, which contributes to the "agent skips interaction" problem pattern.
Suggested: Use agent-agnostic language: "Present the list and ask the user to select" or "Ask the user which change to refine".
(This is a known cross-cutting issue — see #258, #819 — but new workflows shouldn't introduce more instances.)
🟠 Should Fix: Missing openspec validate --strict in the refine loop
After applying each fix in Step 6, the workflow should validate the updated artifact structurally:
openspec validate --change "<name>" --strict
This catches structural issues (missing required fields, invalid YAML frontmatter, etc.) that semantic review might miss. Without it, refine can commit structurally invalid artifacts. CodeRabbit flagged this too.
🟡 Suggestion: Cross-check against existing main specs
The original #783 request specifically called out "Duplication with existing specs/components" as a key check. The current implementation only cross-checks within the change's own artifacts. Consider adding a step (or at least a note in graceful degradation) that checks against openspec/main-specs/ if it exists. This was one of the most valuable aspects of your original custom skill.
🟢 What's Good
buildInstructionspattern — Clean DRY split between skill/command input phrasing. Other template authors should follow this.- Scratchpad format — Well-structured with status legend, key references, and working constraints. The per-change scoping is correct.
- Graceful degradation — Handling partial artifact sets is thoughtful and necessary.
- Priority system (P0/P1/P2) — Good categorization for issue severity.
- Test coverage — Parity hashes, template count updates, all existing tests passing.
- Documentation — The
docs/commands.mdaddition is thorough with a realistic example. - Prior art attribution — Nice to see @demianmnave's NHC refine command credited.
Bottom line: This is a high-quality contribution from a contributor who deeply understands the problem space. The profiles.ts gap is the only blocker — the other items are improvements that would make the workflow more robust across the agent ecosystem.
Looking forward to seeing this land! 🚀
- Register 'refine' in ALL_WORKFLOWS (profiles.ts) and WORKFLOW_TO_SKILL_DIR (profile-sync-drift.ts) — fixes the blocker where custom profile users couldn't select refine - Replace Cursor-specific 'AskUserQuestion tool' with agent-agnostic phrasing - Add 'openspec validate --strict' step in the refine loop before commits - Update profiles test (11 → 12, add 'refine' to expected list) - Update parity test hashes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/core/profiles.test.ts (1)
23-24: Avoid hardcoding the workflow count literal.Line 24 duplicates cardinality already implied by the expected ID list, which can drift on future workflow additions/removals. Consider deriving length from a shared
expectedarray in the same assertion block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/profiles.test.ts` around lines 23 - 24, The test currently hardcodes the expected workflow count (12); change the assertion to derive the length from the shared expected ID list instead: use the same array used to assert IDs (e.g., expected or expectedIDs) and replace expect(ALL_WORKFLOWS).toHaveLength(12) with expect(ALL_WORKFLOWS).toHaveLength(expected.length) (or expectedIDs.length) so the test stays correct when workflows are added/removed and references the unique symbols ALL_WORKFLOWS and the expected ID array in the test block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/core/profiles.test.ts`:
- Around line 27-33: Add a parity guard test to ensure every workflow id in
ALL_WORKFLOWS has corresponding prompt metadata in WORKFLOW_PROMPT_META: update
the test suite in profiles.test.ts to iterate ALL_WORKFLOWS and assert each id
exists as a key in WORKFLOW_PROMPT_META (e.g., compare ALL_WORKFLOWS against
Object.keys(WORKFLOW_PROMPT_META) or assert every id satisfies id in
WORKFLOW_PROMPT_META), so missing entries like 'refine' will fail the test and
force metadata sync.
---
Nitpick comments:
In `@test/core/profiles.test.ts`:
- Around line 23-24: The test currently hardcodes the expected workflow count
(12); change the assertion to derive the length from the shared expected ID list
instead: use the same array used to assert IDs (e.g., expected or expectedIDs)
and replace expect(ALL_WORKFLOWS).toHaveLength(12) with
expect(ALL_WORKFLOWS).toHaveLength(expected.length) (or expectedIDs.length) so
the test stays correct when workflows are added/removed and references the
unique symbols ALL_WORKFLOWS and the expected ID array in the test block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbafc4f1-801b-4570-9e26-97c6de6dc224
📒 Files selected for processing (5)
src/core/profile-sync-drift.tssrc/core/profiles.tssrc/core/templates/workflows/refine-change.tstest/core/profiles.test.tstest/core/templates/skill-templates-parity.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/core/profiles.ts
- src/core/profile-sync-drift.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/core/templates/skill-templates-parity.test.ts
- src/core/templates/workflows/refine-change.ts
Comment on lines
27
to
33
| it('should contain expected workflow IDs', () => { | ||
| const expected = [ | ||
| 'propose', 'explore', 'new', 'continue', 'apply', | ||
| 'ff', 'sync', 'archive', 'bulk-archive', 'verify', 'onboard', | ||
| 'ff', 'sync', 'archive', 'bulk-archive', 'verify', 'refine', 'onboard', | ||
| ]; | ||
| expect([...ALL_WORKFLOWS]).toEqual(expected); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify 'refine' is in ALL_WORKFLOWS and whether prompt metadata keys match workflow IDs.
python - <<'PY'
import re
from pathlib import Path
profiles = Path("src/core/profiles.ts").read_text(encoding="utf-8")
config = Path("src/commands/config.ts").read_text(encoding="utf-8")
arr = re.search(r'ALL_WORKFLOWS\s*=\s*\[([^\]]+)\]', profiles, re.S)
workflows = re.findall(r"'([^']+)'", arr.group(1)) if arr else []
obj = re.search(r'WORKFLOW_PROMPT_META\s*:\s*Record<[^>]+>\s*=\s*\{(.*?)\n\};', config, re.S)
keys = set()
if obj:
body = obj.group(1)
keys.update(re.findall(r"^\s*'([^']+)'\s*:", body, re.M))
keys.update(re.findall(r'^\s*([A-Za-z][\w-]*)\s*:', body, re.M))
missing = sorted(set(workflows) - keys)
extra = sorted(keys - set(workflows))
print("ALL_WORKFLOWS:", workflows)
print("WORKFLOW_PROMPT_META keys:", sorted(keys))
print("Missing in WORKFLOW_PROMPT_META:", missing)
print("Extra keys not in ALL_WORKFLOWS:", extra)
# Expected: both lists empty.
PYRepository: Fission-AI/OpenSpec
Length of output: 464
Add a guard for workflow prompt metadata parity with ALL_WORKFLOWS.
This test asserts 'refine' in ALL_WORKFLOWS, but metadata is out of sync: 'refine' is missing from WORKFLOW_PROMPT_META keys. A workflow in ALL_WORKFLOWS without prompt metadata cannot be selected in the UI. Add a test that verifies all workflows in ALL_WORKFLOWS have corresponding entries in WORKFLOW_PROMPT_META.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/core/profiles.test.ts` around lines 27 - 33, Add a parity guard test to
ensure every workflow id in ALL_WORKFLOWS has corresponding prompt metadata in
WORKFLOW_PROMPT_META: update the test suite in profiles.test.ts to iterate
ALL_WORKFLOWS and assert each id exists as a key in WORKFLOW_PROMPT_META (e.g.,
compare ALL_WORKFLOWS against Object.keys(WORKFLOW_PROMPT_META) or assert every
id satisfies id in WORKFLOW_PROMPT_META), so missing entries like 'refine' will
fail the test and force metadata sync.
shohu
commented
Mar 31, 2026
Author
shohu
left a comment
shohu
left a comment
There was a problem hiding this comment.
Self-review after maintainer feedback
All feedback from @alfred-openspec has been addressed in the latest push (fc95a3f → e5cb29c):
Must Fix (done)
refineregistered inALL_WORKFLOWS(profiles.ts) andWORKFLOW_TO_SKILL_DIR(profile-sync-drift.ts)
Should Fix (done)
- Replaced Cursor-specific
AskUserQuestion toolwith agent-agnostic phrasing - Added
openspec validate --change "<name>" --strictstep in the refine loop before commits
Suggestion (deferred)
- Cross-check against existing main specs: good idea for a follow-up, keeping this PR focused on the core refine workflow
Review notes
- Enum completeness verified:
'refine'added to all 5 registration points (profiles, sync-drift, skill-generation skill array, command array, skill-templates export) - All 1341 tests passing
- TypeScript type check clean (
tsc --noEmit)
alfred-openspec
approved these changes
Mar 31, 2026
Collaborator
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
All feedback addressed cleanly in e5cb29c:
- ✅
refineregistered inALL_WORKFLOWS(profiles.ts) andWORKFLOW_TO_SKILL_DIR(profile-sync-drift.ts) - ✅ Cursor-specific
AskUserQuestionreplaced with agent-agnostic phrasing - ✅
openspec validate --change "<name>" --strictadded to refine loop (step 6) - ✅ Tests updated (11→12), parity hashes added
- ✅ Graceful degradation, DRY
buildInstructionspattern, proper exports
Clean contribution. Nice work @shohu 🎉
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.
Summary
Adds
/opsx:refine— a new workflow command that reviews change artifacts for cross-artifact consistency, best-practice alignment, and implementation readiness before/opsx:apply.scratchpad.md) in the change directoryPrior art
Inspired by @demianmnave's nhc-opsx-refine command, shared in #783. Key ideas adopted:
Changes
src/core/templates/workflows/refine-change.tssrc/core/templates/skill-templates.tssrc/core/shared/skill-generation.tsdocs/commands.md/opsx:refinedocumentationtest/core/templates/skill-templates-parity.test.tstest/core/shared/skill-generation.test.tsTest plan
tsc --noEmit)openspec updategenerates refine skill files/opsx:refinecreates scratchpad and walks through issuesCloses #783
Summary by CodeRabbit
New Features
/opsx:refinecommand (and corresponding skill) for cross‑artifact refinement: prioritized P0/P1/P2 issues, one‑at‑a‑time fixes with user approval, validation, and committing a tracked scratchpad alongside artifact updates.Documentation
Tests