Skip to content

Commit fb19699

Browse files
hsusulclay-goodclaude
authored
fix(adapters): escape YAML frontmatter values consistently across all command adapters (#1447)
* fix(adapters): escape YAML frontmatter values consistently across all command adapters * fix(yaml): safely double-quote all frontmatter string values and expand table-driven adapter coverage * fix(archive): make command and bulk archive paths root-aware, synchronous, and verified * fix(archive): honor bulk sync inclusion decisions * fix(adapters): honor caller delta subsets, escape control characters, close test gaps Adversarial review of the merged branch turned up four gaps: - Bulk archive tells the sync workflow to ignore `excludedDeltas`, but main's sync-specs calls `existingOutputPaths` the "complete list" of delta specs. An agent following both would sync the delta the caller withheld, step 8b would not catch it (it verifies only included deltas), and the run would still report `sync skipped`. Sync now honors a caller-supplied subset, mirroring the inline rule-snapshot handoff main already added. - escapeYamlValue left C0/DEL/C1 control characters raw. The repo's own parser accepts them, so tests passed while stricter parsers used by other tools reject the document. Emit them as \xHH. - The adapter matrix was a hand-maintained list driving only `description`, so raw interpolation in lingma's name/category/tags — and any newly registered adapter — passed green. It now derives from the registry and drives every string field. - Four bulk-archive template lines were guarded only by golden hashes, which this repo regenerates as routine. Also corrects the escapeYamlValue docstring, which still described the pre-PR conditional-quoting behavior, and adds the missing changeset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(templates): iterate the selected delta subset, not the full CLI list A second adversarial pass found the previous fix incomplete. Narrowing step 3 ("Find delta specs") left step 4 — the loop that actually applies the changes — still reading "for each capability delta spec path returned by the CLI". An agent treating step 3 as descriptive and step 4 as operative re-widens to the full list and syncs the delta bulk archive withheld: the original defect, one step further down the template. Step 4 now iterates the step-3 selection, and the parity test pins both the new wording and the absence of the old. Also from that pass: - Generalize the carve-out beyond archive. It was conditioned on "archive invoked this workflow inline", so a user asking /opsx:sync to sync one delta read as an instruction to ignore them. - Define the two undefined edges: a named path outside existingOutputPaths, and an empty named list. Both stop and report rather than proceeding on a guess. - Drive control characters through the adapter matrix. It drove none, so the escaping this suite exists to prove had no adapter-level coverage and the raw-CR assertion could never fail. Verified live by mutation. - Give contentDerivedFields two markers that differ in length and shape. Same-shaped markers render identically for a length- or slice-derived field, which would drop it from every assertion silently. Drops the `not.toContain('complete list of delta spec files')` assertion: it banned one exact synonym while any reword of the same conflicting instruction passed, so it read as coverage without being it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d32d49f commit fb19699

35 files changed

Lines changed: 675 additions & 257 deletions
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@fission-ai/openspec': patch
3+
---
4+
5+
Generated tool command files now carry valid YAML frontmatter for every supported tool. Command names ship as `OPSX: Explore`, and the unquoted `name: OPSX: Explore` that adapters emitted is not parseable YAML — strict parsers rejected the whole file, so the command failed to load. Several adapters also re-implemented their own escaping, and a few interpolated descriptions in raw.
6+
7+
Escaping now lives in one place (`escapeYamlValue` / `formatTagsArray`) and every adapter uses it. String frontmatter values are always double-quoted, which also keeps values like `true`, `null` and `123` from round-tripping as booleans, nulls and numbers. Non-string fields such as `allowed-tools` and `invokable` are unchanged. Expect the first `openspec update` after upgrading to rewrite the frontmatter lines of your generated command files.
8+
9+
Archive workflow guidance also gets two corrections: bulk archive now carries its per-delta include/exclude decisions into execution, so a delta whose implementation was not found is reported as `sync skipped` instead of being synced anyway, and both archive workflows verify the main specs before moving the change directory.

skills/openspec-bulk-archive-change/SKILL.md

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,9 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig
106106
- If neither implemented -> skip spec sync, warn user
107107

108108
d. **Record resolution** for each conflict:
109-
- Which change's specs to apply
110-
- In what order (if both)
109+
- An inclusion or exclusion decision for every delta spec, keyed by change and capability
110+
- Which included delta specs to apply and in what order
111+
- Which delta specs to exclude from sync because their implementation is missing
111112
- Rationale (what was found in codebase)
112113

113114
6. **Show consolidated status table**
@@ -151,7 +152,7 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig
151152
so match what the user picked rather than the wording above:
152153
- "Cancel" — stop, do not archive. Report that nothing was archived and skip the remaining steps.
153154
- The archive-everything option — proceed with every selected change
154-
- The ready-only option — proceed with only the changes the step 6 table marks `Ready` or `Ready*`, and record the rest as Skipped in step 8c. If a `Ready*` change's conflict partner is skipped, re-derive that conflict's resolution using only the changes being archived.
155+
- The ready-only option — proceed with only the changes the step 6 table marks `Ready` or `Ready*`, and record the rest as Skipped in step 8d. If a `Ready*` change's conflict partner is skipped, re-derive that conflict's resolution using only the changes being archived.
155156
- Anything else — ask again rather than archiving
156157

157158
Before step 8 writes the first main spec or moves any change, fetch every
@@ -166,19 +167,35 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig
166167

167168
8. **Execute archive for each confirmed change**
168169

170+
Before processing, carry the recorded decisions from step 5 (after any step 7 re-derivation) into two per-delta sets:
171+
- `includedDeltas`: all non-conflicting delta specs from confirmed changes plus conflict deltas selected for sync
172+
- `excludedDeltas`: conflict deltas from confirmed changes excluded because their implementation is missing
173+
- A single change can have both included and excluded delta specs. Keep the decision per delta; do not collapse it into a per-change sync flag.
174+
169175
Process changes in the determined order (respecting conflict resolution):
170176

171-
a. **Sync specs** if delta specs exist:
172-
- Use the openspec-sync-specs approach (agent-driven intelligent merge)
173-
- For conflicts, apply in resolved order
177+
a. **Sync included delta specs**:
178+
- Run the `openspec-sync-specs` workflow inline (agent-driven intelligent merge) only for changes with entries in `includedDeltas`, passing only the included delta paths and explicitly instructing it to ignore that change's `excludedDeltas`. Wait for it to finish.
179+
- For conflicts, apply in resolved order.
174180
- Pass that change's fetched specs-rule snapshot into inline sync; inline
175181
sync must reuse it without fetching instructions again
176182
- Apply artifact rules only to main specs produced by that change. They do
177183
not change conflict resolution, archive behavior, or CLI contracts, and
178184
their text is not copied into an output file
179-
- Track if sync was done
185+
- Do not delegate to a background task — step 8c would move `changeRoot` out from under a sync that is still reading it.
186+
- If a change has no included delta specs, do not run the sync workflow for it.
187+
188+
b. **Verify included delta specs before moving changeRoot**:
189+
- Re-run the comparison only for delta specs in `includedDeltas` against main spec at `<planningHome.root>/openspec/specs/<capability>/spec.md` (use the store-aware `planningHome.root` from step 3 status JSON, not a hardcoded repo path).
190+
- Verify that main specs are updated:
191+
- ADDED requirements present
192+
- MODIFIED requirements carrying scenario and description changes named in the delta, with their other scenarios intact
193+
- REMOVED requirements gone
194+
- RENAMED requirements present under the new name and absent under the old one
195+
- Do not verify delta specs in `excludedDeltas`; they are intentionally left unsynced.
196+
- If sync failed or any capability does not match verification, report what differs and fail/skip moving that change's `changeRoot` — do not archive that change. `changeRoot` remains intact.
180197

181-
b. **Perform the archive**:
198+
c. **Perform the archive**:
182199

183200
Target name: use the change name as-is when it already starts with a `YYYY-MM-DD-` prefix; otherwise prepend the current date as `YYYY-MM-DD-<name>` (same rule as `openspec archive`).
184201

@@ -187,10 +204,11 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig
187204
mv "<changeRoot>" "<planningHome.changesDir>/archive/<target-name>"
188205
```
189206

190-
c. **Track outcome** for each change:
207+
d. **Track outcome** for each change:
191208
- Success: archived successfully
192-
- Failed: error during archive (record error)
209+
- Failed: error during archive or spec verification (record error)
193210
- Skipped: user chose not to archive (if applicable)
211+
- Sync skipped: for every delta in `excludedDeltas`, report `sync skipped` with the change, capability, and recorded reason. This is distinct from skipping the archive.
194212

195213
9. **Display summary**
196214

@@ -209,7 +227,8 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig
209227
210228
Spec sync summary:
211229
- 4 delta specs synced to main specs
212-
- 1 conflict resolved (auth: applied both in chronological order)
230+
- 1 delta spec sync skipped (add-jwt/auth: implementation not found)
231+
- 1 conflict resolved (auth: synced add-oauth, skipped add-jwt)
213232
```
214233

215234
If any failures:
@@ -222,7 +241,7 @@ This skill allows you to batch-archive changes, handling spec conflicts intellig
222241

223242
Example 1: Only one implemented
224243
```text
225-
Conflict: specs/auth/spec.md touched by [add-oauth, add-jwt]
244+
Conflict: <planningHome.root>/openspec/specs/auth/spec.md touched by [add-oauth, add-jwt]
226245
227246
Checking add-oauth:
228247
- Delta adds "OAuth Provider Integration" requirement
@@ -237,7 +256,7 @@ Resolution: Only add-oauth is implemented. Will sync add-oauth specs only.
237256

238257
Example 2: Both implemented
239258
```text
240-
Conflict: specs/api/spec.md touched by [add-rest-api, add-graphql]
259+
Conflict: <planningHome.root>/openspec/specs/api/spec.md touched by [add-rest-api, add-graphql]
241260
242261
Checking add-rest-api (created 2026-01-10):
243262
- Delta adds "REST Endpoints" requirement
@@ -301,6 +320,10 @@ No active changes found. Create a new change to get started.
301320
- Preserve .openspec.yaml when moving to archive
302321
- Archive directory target uses current date: YYYY-MM-DD-<name>; a name that already starts with a `YYYY-MM-DD-` prefix is used as-is (never stack a second date)
303322
- If archive target exists, fail that change but continue with others
323+
- If sync is requested, run the `openspec-sync-specs` workflow inline (agent-driven) for each change with included delta specs
324+
- Carry the per-delta `includedDeltas` and `excludedDeltas` decisions into execution; sync and verify only included deltas
325+
- Report every excluded delta as `sync skipped` without treating the archive itself as skipped
326+
- Never archive a change while a spec sync is still in flight — run the sync inline and verify main specs at `<planningHome.root>/openspec/specs/<capability>/spec.md` before moving `changeRoot`
304327
- Fetch archive inputs once per selected root before spec inspection or moves
305328
- Fetch all required specs-rule snapshots before the batch's first main-spec write or move
306329
- A failed archive-inputs lookup never blocks the batch; it proceeds with no context or guidance

skills/openspec-sync-specs/SKILL.md

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,23 @@ This is an **agent-driven** operation - you will read delta specs and directly e
3939
3. **Find delta specs**
4040

4141
Use `artifactPaths.specs.existingOutputPaths` from the status JSON as the
42-
complete list of delta spec files. If the `specs` entry is missing or
42+
only source of delta spec paths. If the `specs` entry is missing or
4343
`existingOutputPaths` is empty, report that there are no delta specs to sync,
4444
do not infer them from other artifacts, and stop without requesting artifact
4545
instructions or writing a main spec.
4646

47+
Sync every path in `existingOutputPaths` unless the caller narrowed the set.
48+
A caller narrows it by naming an explicit list of delta spec paths to sync —
49+
archive does this inline, and a user can too ("only sync the billing delta").
50+
Then sync only the named paths and leave the remaining delta specs untouched:
51+
bulk archive excludes a delta whose implementation it could not find, and
52+
syncing it anyway would write a main spec the caller deliberately withheld.
53+
Carry that narrowed selection through step 4; never widen it back to the full
54+
list. If a named path is not in `existingOutputPaths`, do not sync it —
55+
report it and stop, rather than dropping it silently. If the named list is
56+
empty, report that there is nothing to sync and stop without writing a main
57+
spec.
58+
4759
Each delta spec file contains sections like:
4860
- `## ADDED Requirements` - New requirements to add
4961
- `## MODIFIED Requirements` - Changes to existing requirements
@@ -70,7 +82,7 @@ This is an **agent-driven** operation - you will read delta specs and directly e
7082
selected roots, delta paths, CLI checks, or workflow steps. Use their text as
7183
constraints without copying it verbatim into a main spec or summary.
7284

73-
For each capability delta spec path returned by the CLI (these may belong to a selected store, not the repo):
85+
For each capability delta spec path selected in step 3 — the full `existingOutputPaths` list, or the narrowed subset when a caller supplied one (these may belong to a selected store, not the repo):
7486

7587
a. **Read the delta spec** to understand the intended changes
7688

@@ -201,6 +213,7 @@ Main specs are now updated. The change remains active - archive when implementat
201213
- Show what you're changing as you go
202214
- The operation should be idempotent - running twice should give same result
203215
- Use only `artifactPaths.specs.existingOutputPaths`; never infer delta specs from unrelated artifacts
216+
- Honor a caller-supplied subset of `existingOutputPaths`; never widen it back to the full list
204217
- Fetch specs instructions once for direct sync, or reuse the archive-supplied snapshot inline
205218
- Stop before every main-spec write on a non-zero or invalid JSON specs-instruction response
206219
- Artifact rules constrain only the specs being written and are never copied into output files

src/core/command-generation/adapters/amazon-q.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9+
import { escapeYamlValue } from '../yaml.js';
910

1011
/**
1112
* Amazon Q adapter for command generation.
@@ -21,7 +22,7 @@ export const amazonQAdapter: ToolCommandAdapter = {
2122

2223
formatFile(content: CommandContent): string {
2324
return `---
24-
description: ${content.description}
25+
description: ${escapeYamlValue(content.description)}
2526
---
2627
2728
${content.body}

src/core/command-generation/adapters/antigravity.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9+
import { escapeYamlValue } from '../yaml.js';
910

1011
/**
1112
* Antigravity adapter for command generation.
@@ -21,7 +22,7 @@ export const antigravityAdapter: ToolCommandAdapter = {
2122

2223
formatFile(content: CommandContent): string {
2324
return `---
24-
description: ${content.description}
25+
description: ${escapeYamlValue(content.description)}
2526
---
2627
2728
${content.body}

src/core/command-generation/adapters/auggie.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9+
import { escapeYamlValue } from '../yaml.js';
910

1011
/**
1112
* Auggie adapter for command generation.
@@ -21,7 +22,7 @@ export const auggieAdapter: ToolCommandAdapter = {
2122

2223
formatFile(content: CommandContent): string {
2324
return `---
24-
description: ${content.description}
25+
description: ${escapeYamlValue(content.description)}
2526
argument-hint: command arguments
2627
---
2728

src/core/command-generation/adapters/bob.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ import { escapeYamlValue } from '../yaml.js';
1313
/**
1414
* Bob Shell adapter for command generation.
1515
* File path: .bob/commands/opsx-<id>.md
16-
* Frontmatter: description, argument-hint
16+
* Frontmatter: description
17+
*
18+
* Bob uses the filename (minus .md) as the slash command name, so
19+
* opsx-propose.md → /opsx-propose. Command references in the body
20+
* are transformed from /opsx: to /opsx- for consistency.
1721
*/
1822
export const bobAdapter: ToolCommandAdapter = {
1923
toolId: 'bob',
@@ -23,7 +27,6 @@ export const bobAdapter: ToolCommandAdapter = {
2327
},
2428

2529
formatFile(content: CommandContent): string {
26-
// Transform command references from colon to hyphen format for Bob
2730
const transformedBody = transformToHyphenCommands(content.body);
2831

2932
return `---

src/core/command-generation/adapters/claude.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,9 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9-
import { escapeYamlValue } from '../yaml.js';
9+
import { escapeYamlValue, formatTagsArray } from '../yaml.js';
1010
import { OPENSPEC_CLI_ALLOWED_TOOLS } from '../../shared/allowed-tools.js';
1111

12-
/**
13-
* Formats a tags array as a YAML array with proper escaping.
14-
*/
15-
function formatTagsArray(tags: string[]): string {
16-
const escapedTags = tags.map((tag) => escapeYamlValue(tag));
17-
return `[${escapedTags.join(', ')}]`;
18-
}
19-
2012
/**
2113
* Claude Code adapter for command generation.
2214
* File path: .claude/commands/opsx/<id>.md

src/core/command-generation/adapters/codebuddy.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9+
import { escapeYamlValue } from '../yaml.js';
910

1011
/**
1112
* CodeBuddy adapter for command generation.
@@ -21,8 +22,8 @@ export const codebuddyAdapter: ToolCommandAdapter = {
2122

2223
formatFile(content: CommandContent): string {
2324
return `---
24-
name: ${content.name}
25-
description: "${content.description}"
25+
name: ${escapeYamlValue(content.name)}
26+
description: ${escapeYamlValue(content.description)}
2627
argument-hint: "[command arguments]"
2728
---
2829

src/core/command-generation/adapters/continue.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9+
import { escapeYamlValue } from '../yaml.js';
910

1011
/**
1112
* Continue adapter for command generation.
@@ -21,8 +22,8 @@ export const continueAdapter: ToolCommandAdapter = {
2122

2223
formatFile(content: CommandContent): string {
2324
return `---
24-
name: opsx-${content.id}
25-
description: ${content.description}
25+
name: ${escapeYamlValue(`opsx-${content.id}`)}
26+
description: ${escapeYamlValue(content.description)}
2627
invokable: true
2728
---
2829

0 commit comments

Comments
 (0)