Skip to content

Improve frontmatter persistence across str-replace/write#25

Merged
lyssieth merged 2 commits into
mainfrom
improve-file-edit-safety
Jun 7, 2026
Merged

Improve frontmatter persistence across str-replace/write#25
lyssieth merged 2 commits into
mainfrom
improve-file-edit-safety

Conversation

@lyssieth

@lyssieth lyssieth commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

There's been some issues with the frontmatter getting decimated by agents.

Well, no more. This PR should un-fuck most cases where it can happen. There are still obviously cases where it can, but we can only cover so much surface-area when it's infinite.

Security-Critical Analysis — Updated

The prior validation gap has been addressed: frontmatter is now parsed and semantically validated before being preserved or accepted.

What changed (security-relevant):

  • Frontmatter parsing & Valibot schemas were centralized in packages/runtime/src/util/frontmatter.ts (BlockFrontmatterSchema, SkillFrontmatterSchema) with a new validateFrontmatter(content, isBlock) function that enforces delimiters, parses TOML/YAML, and runs Valibot validation.
  • write.ts validates preserved frontmatter from existing block/skill files before prepending it to new content, and also validates agent-provided frontmatter on creation of new block/skill files (when the content starts with the expected delimiter).
  • str-replace.ts validates preserved frontmatter before writing when it extracted frontmatter for scoped body edits.
  • load.ts now imports and uses the shared schemas (BlockFrontmatterSchema, SkillFrontmatterSchema) for runtime loading/validation.

Impact:

  • The risk that malformed frontmatter could be silently preserved by write/str-replace and cause downstream load-time failures has been mitigated because preserved frontmatter is validated prior to write.
  • Agents still can produce invalid frontmatter, but they will now receive immediate validation errors during write/str-replace operations rather than causing later load failures.

Other security notes:

  • Path isolation and sandbox path checks are unchanged by this PR.
  • Parsing/validation behavior is stricter and centralized, improving consistency and reducing the previously-identified attack surface where corrupted frontmatter could propagate silently.

Recommendation:

  • None required immediately; the PR implements the recommended validation. Continue to monitor tests covering failure modes (invalid/missing fields, malformed TOML/YAML) added with the changes.

@lyssieth lyssieth self-assigned this Jun 7, 2026
@lyssieth lyssieth added the enhancement New feature or request label Jun 7, 2026
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a988e80e-1e0a-4ec5-b708-fe92aebc8ee0

📥 Commits

Reviewing files that changed from the base of the PR and between c315934 and 9fe6d0d.

📒 Files selected for processing (7)
  • packages/runtime/src/engine/tools/str-replace.test.ts
  • packages/runtime/src/engine/tools/str-replace.ts
  • packages/runtime/src/engine/tools/write.test.ts
  • packages/runtime/src/engine/tools/write.ts
  • packages/runtime/src/util/frontmatter.test.ts
  • packages/runtime/src/util/frontmatter.ts
  • packages/runtime/src/util/load.ts

📝 Walkthrough

Walkthrough

This PR implements transparent frontmatter preservation for block and skill files. A new utility module detects which paths require frontmatter and parses delimited sections. The str-replace and write tools now use these utilities to isolate text operations to file bodies while preserving existing frontmatter during overwrites.

Changes

Frontmatter preservation for block and skill files

Layer / File(s) Summary
Frontmatter utility module and tests
packages/runtime/src/util/frontmatter.ts, packages/runtime/src/util/frontmatter.test.ts
requiresFrontmatter classifies paths matching /blocks/... and /skills/**/SKILL.md as requiring frontmatter. splitFrontmatter parses TOML (+++...+++) or YAML (---...---) delimiters and returns { body, frontmatter } or undefined if delimiters are invalid or missing. Tests validate path classification and parsing for both modes.
str-replace tool frontmatter isolation
packages/runtime/src/engine/tools/str-replace.ts, packages/runtime/src/engine/tools/str-replace.test.ts
When frontmatter is required, str-replace splits file content, searches and replaces only in the body, and reconstructs the file with preserved frontmatter. Context line offsets are adjusted to exclude frontmatter rows. Tests validate body-only replacement, preservation of existing frontmatter, rejection of frontmatter-only text matches, and correct context positioning.
write tool existing frontmatter preservation
packages/runtime/src/engine/tools/write.ts, packages/runtime/src/engine/tools/write.test.ts
When overwriting existing files that require frontmatter, write reads the existing file, extracts its frontmatter, and conditionally prepends it to new content if the new content lacks the expected opening delimiter. Tests validate frontmatter is preserved for existing block/skill files, not prepended when new content already includes frontmatter, not added for new files, and not applied to non-block/skill paths.
load.ts frontmatter schema centralization
packages/runtime/src/util/load.ts
Replaces a previously local frontmatter schema with the shared BlockFrontmatterSchema and SkillFrontmatterSchema imports and updates loadSkills to use the shared skill schema. The export list no longer includes the old Frontmatter type.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main objective of the pull request: improving how frontmatter is preserved when using str-replace and write tools.
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.


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

@lyssieth lyssieth force-pushed the improve-file-edit-safety branch from ee1f9d1 to f571f28 Compare June 7, 2026 03:42
@lyssieth lyssieth force-pushed the improve-file-edit-safety branch from f571f28 to c315934 Compare June 7, 2026 03:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@packages/runtime/src/engine/tools/str-replace.ts`:
- Around line 62-75: The code suppresses the prefer-destructuring lint rule when
assigning frontmatter and body from splitFrontmatter; replace the manual
assignment with a destructuring assignment to avoid the linter suppression: when
split = splitFrontmatter(...) returns a value, use ({ frontmatter, body:
searchContent } = split) instead of assigning split.frontmatter and split.body
separately; update the block around requiresFrontmatter, splitFrontmatter,
frontmatter and searchContent to use this destructuring and remove the
oxlint-disable-next-line comment.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 519d6989-8b03-4ab6-a4d4-f3f40ad46451

📥 Commits

Reviewing files that changed from the base of the PR and between 1a21317 and c315934.

📒 Files selected for processing (6)
  • packages/runtime/src/engine/tools/str-replace.test.ts
  • packages/runtime/src/engine/tools/str-replace.ts
  • packages/runtime/src/engine/tools/write.test.ts
  • packages/runtime/src/engine/tools/write.ts
  • packages/runtime/src/util/frontmatter.test.ts
  • packages/runtime/src/util/frontmatter.ts

Comment thread packages/runtime/src/engine/tools/str-replace.ts
When str-replace or write preserves frontmatter (blocks: TOML, skills: YAML),
validate it against the Valibot schema before persisting. Catches corruption
early with an actionable error rather than a mysterious load failure later.

- Move BlockFrontmatterSchema and SkillFrontmatterSchema from load.ts to
  frontmatter.ts so validation and parsing share one source of truth.
- Add validateFrontmatter(content, isBlock) to frontmatter.ts — parses with
  smol-toml / yaml and runs vb.parse against the matching schema.
- Wire into str-replace.ts (preserved frontmatter) and write.ts (preserved +
  agent-provided frontmatter for new files).
- Also: replace prefer-destructuring lint suppression with proper destructuring
  in str-replace.ts.

Tests: 13 new (11 validateFrontmatter unit tests, 4 tool integration tests).
@lyssieth lyssieth force-pushed the improve-file-edit-safety branch from f078377 to 9fe6d0d Compare June 7, 2026 03:53
@lyssieth lyssieth merged commit 468f70b into main Jun 7, 2026
7 checks passed
@lyssieth lyssieth deleted the improve-file-edit-safety branch June 7, 2026 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant