fix(schema): preserve YAML formatting when forking a schema

[linjinze999]

Use yaml's Document API (parseDocument + doc.set) instead of round -tripping through parseSchema/stringifyYaml so block scalars, comments and key order in the source schema.yaml survive the fork

Fix yaml：

• A: |
  xxx
  yyy
  will be：
• A: xxx yyy

Summary by CodeRabbit

• Refactor
  • Enhanced the schema forking process with improved internal handling of schema name updates during schema operations.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors how the schema fork command updates a schema's name in its YAML configuration file. It switches from parsing the YAML into a typed schema model, mutating the model, and re-stringifying it—to parsing the YAML as a document, using targeted field mutation, and converting back to a string.

Changes

YAML Document-Level Editing

Layer / File(s)	Summary
YAML document-level field mutation src/commands/schema.ts	Import parseDocument from the YAML library and replace schema-model–based name field updates in schema fork with document-level doc.set('name', ...) and doc.toString() to enable targeted field edits.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A fork once parsed the whole schema through,
But now we set just one field—clean and true!
Document-level edits, precise and lean,
The name updates faster, the structure stays keen.
Hop, hop, refactor! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(schema): preserve YAML formatting when forking a schema' directly and clearly describes the main change: using YAML's Document API to preserve formatting (block scalars, comments, key order) when forking schemas instead of round-tripping through parseSchema/stringifyYaml.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/commands/schema.ts (1)

629-631: ⚡ Quick win

Don’t worry about silent corruption: toString() already fails on YAML parse errors

• yaml (eemeli/yaml) Document#toString() throws when doc.errors.length > 0, so fs.writeFileSync(...) won’t run and the existing try/catch will prevent overwriting an invalid schema.yaml.
• An explicit doc.errors check is still a useful optional improvement for clearer error messaging and to avoid mutating via doc.set.

Proposed fix

 const doc = parseDocument(schemaContent);
+if (doc.errors.length > 0) {
+  throw new Error(
+    `Failed to parse '${destSchemaPath}': ${doc.errors.map((e) => e.message).join('; ')}`
+  );
+}
 doc.set('name', destinationName);
 fs.writeFileSync(destSchemaPath, doc.toString());

🤖 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/commands/schema.ts` around lines 629 - 631, The code currently calls
parseDocument(schemaContent) then mutates the Document with doc.set('name',
destinationName) before writing, which can silently mutate an invalid Document;
update the flow in the block that uses parseDocument, Document#toString, and
fs.writeFileSync so you first check doc.errors (after parseDocument) and if any
errors exist throw or surface a clear error message, and only then call
doc.set('name', destinationName) and fs.writeFileSync(destSchemaPath,
doc.toString()); this ensures you don’t mutate an invalid Document and that
toString() won’t be relied on to detect parse problems.

🤖 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.

Nitpick comments:
In `@src/commands/schema.ts`:
- Around line 629-631: The code currently calls parseDocument(schemaContent)
then mutates the Document with doc.set('name', destinationName) before writing,
which can silently mutate an invalid Document; update the flow in the block that
uses parseDocument, Document#toString, and fs.writeFileSync so you first check
doc.errors (after parseDocument) and if any errors exist throw or surface a
clear error message, and only then call doc.set('name', destinationName) and
fs.writeFileSync(destSchemaPath, doc.toString()); this ensures you don’t mutate
an invalid Document and that toString() won’t be relied on to detect parse
problems.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e76ea6c6-73d8-48bc-bf75-41ddf0f74f7e

📥 Commits

Reviewing files that changed from the base of the PR and between e441287 and 2485f8d.

📒 Files selected for processing (1)

• src/commands/schema.ts

[alfred-openspec]

The Document API preserves formatting, but this change also removes the structural parseSchema validation from schema fork: I reproduced an invalid schema missing required artifact fields being serialized by doc.set/toString while the old path rejects it. Please keep schema validation before the document mutation and add a fork-level regression covering both formatting preservation and invalid-source rejection.

[clay-good]

Thanks for this fix, @linjinze999 — the Document-API approach is the right call. I've carried it forward in #1607, which keeps your change and additionally restores the parseSchema structural validation that the alfred-openspec review asked for (so an invalid source schema is still rejected), plus adds a fork-level regression test covering both formatting preservation and invalid-source rejection. Your authorship is preserved via co-author. Closing in favor of #1607.