fix(openspec): refresh against new v1.x workflow modules#1047
fix(openspec): refresh against new v1.x workflow modules#1047pedramamini wants to merge 2 commits into
Conversation
OpenSpec restructured its repo around v1.0: workflow prompts moved out of the monolithic openspec/AGENTS.md (which is now empty at the repo root) into per-workflow TypeScript modules under src/core/templates/workflows/. The old refresh logic still pointed at openspec/AGENTS.md, so every "Check for Updates" 404'd and left users pinned to bundled v0.19.0 prompts. - Rewrite refreshOpenSpecPrompts and the refresh-openspec script to fetch new-change.ts / apply-change.ts / archive-change.ts and extract the `instructions` template literal from each. - Re-bundle the v1.3.1 prompts so fresh installs ship up-to-date defaults instead of v0.19.0. - Update tests for the new fetch shape. Closes #1046
📝 WalkthroughWalkthroughThis PR updates the OpenSpec prompt refresh mechanism to fetch per-workflow TypeScript modules from the upstream repository and extract their ChangesOpenSpec TS Module Extraction and Prompts
Sequence Diagram(s)sequenceDiagram
participant RefreshScript as refresh-openspec.mjs
participant GitHubAPI as GitHub API
participant RawGitHub as Raw GitHub Content
participant FSWrite as Filesystem
participant ManagerTs as openspec-manager.ts
RefreshScript->>GitHubAPI: Fetch latest release tag
GitHubAPI-->>RefreshScript: Return v1.3.1 or fallback to main
RefreshScript->>RawGitHub: Fetch proposal.ts, apply.ts, archive.ts
RawGitHub-->>RefreshScript: Return TypeScript module content
RefreshScript->>RefreshScript: Extract instructions: `...` from each TS
RefreshScript->>FSWrite: Write openspec.proposal.md, openspec.apply.md, openspec.archive.md
RefreshScript->>FSWrite: Write metadata.json with v1.3.1, lastRefreshed, commitSha
ManagerTs->>RawGitHub: Fetch workflow modules via same flow
ManagerTs->>ManagerTs: extractInstructions() and regex match
ManagerTs->>FSWrite: Write user prompt files with extracted content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR adapts the OpenSpec prompt-refresh mechanism from parsing a single monolithic
Confidence Score: 4/5Safe to merge; the new fetch strategy, regex extraction, and unescape logic are all correct and well-tested. The core rewrite is clean: the regex correctly handles multi-line template literals and backtick escapes, the four-call fetch shape is tested end-to-end including URL assertions, and the bundled v1.3.1 prompts match what the script produces. The noted issues are both in partial-failure handling — the .mjs script can exit 0 and write versioned metadata when one prompt was unchanged and others failed, and the runtime manager logs no warning before returning when fewer than all three prompts were written — neither affects the normal happy path. scripts/refresh-openspec.mjs — the exit condition for total failure is slightly too permissive; src/main/openspec-manager.ts — partial-update failures are silently swallowed before returning metadata. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User / CI
participant M as openspec-manager.ts
participant GH as GitHub API
participant FS as Filesystem
U->>M: refreshOpenSpecPrompts()
M->>GH: GET /repos/Fission-AI/OpenSpec/releases/latest
GH-->>M: "{ tag_name: "v1.3.1" }"
loop for each workflow (new-change, apply-change, archive-change)
M->>GH: "GET raw.githubusercontent.com/.../v1.3.1/src/core/templates/workflows/<file>.ts"
GH-->>M: TypeScript source
M->>M: extractInstructions() — regex + unescape
M->>FS: "writeFile openspec.<cmdId>.md"
end
M->>FS: "writeFile metadata.json (version=v1.3.1)"
M-->>U: OpenSpecMetadata
Reviews (1): Last reviewed commit: "add image" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (updatedCount === 0 && failures.length === Object.keys(UPSTREAM_WORKFLOWS).length) { | ||
| console.error('❌ Failed to fetch any workflow prompts'); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
The updatedCount === 0 && failures.length === all guard won't trigger process.exit(1) when one workflow file happens to be unchanged (content matches what's already on disk, so updatedCount is never incremented) while the other two fail to fetch. In that scenario updatedCount = 0 and failures.length = 2 < 3, so the condition is false, the script exits 0, and metadata.json is written claiming the full new version even though 2/3 prompts weren't refreshed. The manager.ts counterpart avoids this because it always writes unconditionally (making updatedCount >= 1 for any successful extraction), but the .mjs script can silently produce a misleadingly-versioned metadata file when run as a CI/release step.
| if (updatedCount === 0) { | ||
| throw new Error( | ||
| `Failed to fetch any OpenSpec workflow prompts from ${version}: ${failures.join(', ')}` | ||
| ); | ||
| } | ||
|
|
||
| // Update metadata with new version info | ||
| const newMetadata: OpenSpecMetadata = { |
There was a problem hiding this comment.
When updatedCount > 0 (at least one prompt was written) but one or more fetches failed, the function proceeds to write metadata.json with the full version tag. A user would see e.g. 1.3.1 in the UI but still be served the bundled (older) defaults for any command whose fetch failed. Consider logging the list of failed filenames at warn-level before returning so callers can surface the partial state.
| if (updatedCount === 0) { | |
| throw new Error( | |
| `Failed to fetch any OpenSpec workflow prompts from ${version}: ${failures.join(', ')}` | |
| ); | |
| } | |
| // Update metadata with new version info | |
| const newMetadata: OpenSpecMetadata = { | |
| if (updatedCount === 0) { | |
| throw new Error( | |
| `Failed to fetch any OpenSpec workflow prompts from ${version}: ${failures.join(', ')}` | |
| ); | |
| } | |
| if (failures.length > 0) { | |
| logger.warn( | |
| `Partially refreshed OpenSpec prompts from ${version}: failed to fetch ${failures.join(', ')}`, | |
| LOG_CONTEXT | |
| ); | |
| } | |
| const newMetadata: OpenSpecMetadata = { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/__tests__/main/openspec-manager.test.ts (1)
704-759: ⚡ Quick winAdd a test for rejected workflow fetch (not just non-OK responses)
Current failure tests cover 404-style responses, but not
fetch()promise rejection for a workflow file. Adding that case will lock in partial-failure behavior and prevent regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/main/openspec-manager.test.ts` around lines 704 - 759, Add a new test case for a rejected workflow fetch by using stubFetch to return a rejected Error for one of the workflow fetch calls (not just non-ok responses), call refreshOpenSpecPrompts(), and assert it still uses 'main' as commitSha/sourceVersion, writes the proposal file, does not write the missing workflow file, and emits logger.warn for the rejected workflow (matching the error message and '[OpenSpec]'); reference the existing test pattern and functions stubFetch and refreshOpenSpecPrompts and the logger.warn expectations to mirror the "missing workflows" test but with one fetch returning new Error('network down') instead of an ok:false response.scripts/refresh-openspec.mjs (1)
78-80: ⚡ Quick winNarrow unescaping to backticks only
extractInstructions()currently usesreplace(/\\([\s\S])/g, '$1')(line 80), which strips the backslash from any escape sequence. The file comment states upstream only uses backslash-escaping for inline-code backticks (```), so narrow the logic to backticks to match intent and avoid unintended future escape corruption.Proposed fix
function extractInstructions(tsContent) { const match = tsContent.match(/instructions:\s*`((?:\\[\s\S]|[^`\\])*)`/); if (!match) return null; - return match[1].replace(/\\([\s\S])/g, '$1'); + return match[1].replace(/\\`/g, '`'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/refresh-openspec.mjs` around lines 78 - 80, The unescape in extractInstructions() is too broad—replace(/\\([\s\S])/g, '$1') removes backslashes from any escape sequence; change it to only unescape backticked inline-code escapes by replacing backslash-backtick sequences (e.g., use a regex that targets `\\\``) so the post-match replacement on the capture from tsContent.match(/instructions:\s*`((?:\\[\s\S]|[^`\\])*)`/) only strips backslash from escaped backticks and leaves other escapes intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/openspec-manager.ts`:
- Around line 370-377: The loop in refreshOpenSpecPrompts currently calls
fetch(url) without a try/catch so any network rejection will throw and abort the
whole refresh; wrap the per-workflow fetch and response handling for each entry
(the code iterating over UPSTREAM_WORKFLOWS and using url, response, failures,
logger.warn) in a try/catch, on error push the filename into failures and call
logger.warn and Sentry.captureException/captureMessage with context (include
cmdId, filename and url), then continue to the next item; keep the existing
non-OK response handling unchanged but move it inside the try block so both HTTP
errors and network rejections are handled similarly.
In `@src/prompts/openspec/openspec.apply.md`:
- Line 83: Three fenced code blocks around the headings "Implementing:
<change-name> (schema: <schema-name>)", "Implementation Complete", and
"Implementation Paused" are missing language identifiers and trigger MD040;
update each opening triple-backtick to include a language (e.g., add "text") so
the fences read ```text for those three blocks in
src/prompts/openspec/openspec.apply.md.
In `@src/prompts/openspec/openspec.archive.md`:
- Around line 86-95: The fenced code block containing the "## Archive Complete"
section is missing a language tag which triggers markdownlint MD040; update the
opening fence from ``` to ```md so the block becomes a Markdown-highlighted
block (target the fenced block that starts with "## Archive Complete" and the
surrounding triple backticks in openspec.archive.md).
---
Nitpick comments:
In `@scripts/refresh-openspec.mjs`:
- Around line 78-80: The unescape in extractInstructions() is too
broad—replace(/\\([\s\S])/g, '$1') removes backslashes from any escape sequence;
change it to only unescape backticked inline-code escapes by replacing
backslash-backtick sequences (e.g., use a regex that targets `\\\``) so the
post-match replacement on the capture from
tsContent.match(/instructions:\s*`((?:\\[\s\S]|[^`\\])*)`/) only strips
backslash from escaped backticks and leaves other escapes intact.
In `@src/__tests__/main/openspec-manager.test.ts`:
- Around line 704-759: Add a new test case for a rejected workflow fetch by
using stubFetch to return a rejected Error for one of the workflow fetch calls
(not just non-ok responses), call refreshOpenSpecPrompts(), and assert it still
uses 'main' as commitSha/sourceVersion, writes the proposal file, does not write
the missing workflow file, and emits logger.warn for the rejected workflow
(matching the error message and '[OpenSpec]'); reference the existing test
pattern and functions stubFetch and refreshOpenSpecPrompts and the logger.warn
expectations to mirror the "missing workflows" test but with one fetch returning
new Error('network down') instead of an ok:false response.
🪄 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: 166ab025-3c3e-4ca1-ba5f-f1296c9dde9f
⛔ Files ignored due to path filters (1)
assets/logo.pngis excluded by!**/*.png
📒 Files selected for processing (9)
scripts/refresh-openspec.mjssrc/__tests__/main/ipc/handlers/openspec.test.tssrc/__tests__/main/openspec-manager.test.tssrc/main/openspec-manager.tssrc/prompts/openspec/index.tssrc/prompts/openspec/metadata.jsonsrc/prompts/openspec/openspec.apply.mdsrc/prompts/openspec/openspec.archive.mdsrc/prompts/openspec/openspec.proposal.md
| for (const [cmdId, filename] of Object.entries(UPSTREAM_WORKFLOWS)) { | ||
| const url = `https://raw.githubusercontent.com/Fission-AI/OpenSpec/${version}/src/core/templates/workflows/${filename}`; | ||
| const response = await fetch(url); | ||
| if (!response.ok) { | ||
| failures.push(`${filename} (${response.status})`); | ||
| logger.warn(`Failed to fetch ${filename}: ${response.statusText}`, LOG_CONTEXT); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify there is no per-item try/catch around workflow fetch calls in refreshOpenSpecPrompts.
set -euo pipefail
rg -n -C3 'for \(const \[cmdId, filename\] of Object\.entries\(UPSTREAM_WORKFLOWS\)\)|await fetch\(url\)' src/main/openspec-manager.tsRepository: RunMaestro/Maestro
Length of output: 545
Handle per-workflow fetch rejections in refreshOpenSpecPrompts
The loop only handles HTTP non-OK responses; a network-level fetch(url) rejection will throw and abort the whole refresh instead of continuing to the next workflow entry.
Proposed fix
for (const [cmdId, filename] of Object.entries(UPSTREAM_WORKFLOWS)) {
const url = `https://raw.githubusercontent.com/Fission-AI/OpenSpec/${version}/src/core/templates/workflows/${filename}`;
- const response = await fetch(url);
+ let response: Response;
+ try {
+ response = await fetch(url);
+ } catch (error) {
+ failures.push(`${filename} (network error)`);
+ logger.warn(`Failed to fetch ${filename}: ${String(error)}`, LOG_CONTEXT);
+ continue;
+ }
if (!response.ok) {
failures.push(`${filename} (${response.status})`);
logger.warn(`Failed to fetch ${filename}: ${response.statusText}`, LOG_CONTEXT);
continue;
}Also ensure caught errors are reported with Sentry utilities (captureException/captureMessage) with enough context.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/openspec-manager.ts` around lines 370 - 377, The loop in
refreshOpenSpecPrompts currently calls fetch(url) without a try/catch so any
network rejection will throw and abort the whole refresh; wrap the per-workflow
fetch and response handling for each entry (the code iterating over
UPSTREAM_WORKFLOWS and using url, response, failures, logger.warn) in a
try/catch, on error push the filename into failures and call logger.warn and
Sentry.captureException/captureMessage with context (include cmdId, filename and
url), then continue to the next item; keep the existing non-OK response handling
unchanged but move it inside the try block so both HTTP errors and network
rejections are handled similarly.
|
|
||
| **Output During Implementation** | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks
Lines 83, 97, and 114 trigger MD040. Please specify a language (e.g., text) for each fenced block.
Proposed fix
-```
+```text
## Implementing: <change-name> (schema: <schema-name>)
@@
-```
+```text
## Implementation Complete
@@
-```
+```text
## Implementation PausedAlso applies to: 97-97, 114-114
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/prompts/openspec/openspec.apply.md` at line 83, Three fenced code blocks
around the headings "Implementing: <change-name> (schema: <schema-name>)",
"Implementation Complete", and "Implementation Paused" are missing language
identifiers and trigger MD040; update each opening triple-backtick to include a
language (e.g., add "text") so the fences read ```text for those three blocks in
src/prompts/openspec/openspec.apply.md.
| ``` | ||
| ## Archive Complete | ||
|
|
||
| **Change:** <change-name> | ||
| **Schema:** <schema-name> | ||
| **Archived to:** openspec/changes/archive/YYYY-MM-DD-<name>/ | ||
| **Specs:** ✓ Synced to main specs (or "No delta specs" or "Sync skipped") | ||
|
|
||
| All artifacts complete. All tasks complete. | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced output block.
The fenced block at Line 86 is missing a language tag, which triggers markdownlint MD040.
Proposed fix
-```
+```md
## Archive Complete
**Change:** <change-name>
**Schema:** <schema-name>
**Archived to:** openspec/changes/archive/YYYY-MM-DD-<name>/
**Specs:** ✓ Synced to main specs (or "No delta specs" or "Sync skipped")
All artifacts complete. All tasks complete.</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/prompts/openspec/openspec.archive.md` around lines 86 - 95, The fenced
code block containing the "## Archive Complete" section is missing a language
tag which triggers markdownlint MD040; update the opening fence from ``` to
```md so the block becomes a Markdown-highlighted block (target the fenced block
that starts with "## Archive Complete" and the surrounding triple backticks in
openspec.archive.md).
1 participant
Summary
openspec/AGENTS.mdis gone and per-workflow prompts now live in TypeScript modules undersrc/core/templates/workflows/(new-change.ts,apply-change.ts,archive-change.ts), each exposing aninstructionstemplate literal.refreshOpenSpecPrompts(and the standalonescripts/refresh-openspec.mjs) to fetch those three files and extract the template-literal body, unescaping the only escape upstream uses (\\\`` → ````).Closes #1046
Test plan
npm run lint(afternpm run build:prompts) — cleannpm run lint:eslint— cleannpx vitest run src/__tests__/main/openspec-manager.test.ts src/__tests__/main/ipc/handlers/openspec.test.ts— 36/36 passnode scripts/refresh-openspec.mjsagainst current upstream — pullsv1.3.1and writes the three workflow prompts0.19.0to1.3.1and the three prompts now show the v1.x artifact-driven workflow textSummary by CodeRabbit
Documentation
Chores