fix: enforce one blank line between concatenated rules by Jercik · Pull Request #28 · Jercik/sync-rules

Jercik · 2026-02-06T16:06:33Z

Summary

enforce exactly one blank line between adjacent rules when generating combined output
normalize boundary newlines so extra/missing spacing is corrected consistently
update sync-related tests to assert the new formatting behavior

Testing

pnpm test -- src/core/concatenate-rules.test.ts src/core/sync.test.ts src/core/sync-global.test.ts

Jercik

Approach Review: This solves the spacing issue, but there’s a simpler/less stateful way to guarantee exactly one blank line between all adjacent rules: normalize each rule’s content (strip leading + trailing newlines) and then join("\n\n"). That avoids boundary-specific logic and makes the invariant obvious regardless of which rule happens to be first/middle/last.

Approach review by Copilot (gpt-5.2)

Jercik · 2026-02-06T16:08:44Z

src/core/concatenate-rules.ts

  }

-  let combinedContent = firstRule.content;
+  let combinedContent = firstRule.content.replace(/\n+$/u, "");


💡 Alternative approach: Consider normalizing each rule independently and then joining: e.g. return rules.map(r => r.content.replace(/^\n+|\n+$/gu, "")).join("\n\n"); This makes the "exactly one blank line" invariant explicit and avoids per-boundary edge cases.

github-actions

Summary: No bugs or security issues found. 1 low observation, 2 info-level notes.

Code review by Claude (opus)

github-actions · 2026-02-06T16:26:49Z

src/core/concatenate-rules.ts

    }

-    combinedContent = `${combinedContent}\n${rule.content}`;
+    combinedContent = `${normalizedCombinedContent}\n\n${normalizedRuleContent}`;


ℹ️ Info: Clean implementation. The strip-then-rejoin approach handles all the boundary normalization cases well—CRLF, multiple trailing newlines, empty rules. The empty-content guards on lines 23-31 correctly prevent orphan \n\n separators when rules are whitespace-only.

github-actions · 2026-02-06T16:26:49Z

src/core/concatenate-rules.ts

@@ -13,12 +14,23 @@ export function concatenateRules(rules: Rule[]): string {
  let combinedContent = firstRule.content;


🟢 Low: The first rule's content is assigned directly without normalization (line 14), which means a single whitespace-only rule like {content: "\n\n"} returns "\n\n" verbatim, while multiple whitespace-only rules collapse to "". The single-rule test on line 12 of the test file confirms this is intentional ("preserves content exactly when only one rule is present"), and in practice rule files won't be pure whitespace, so this is fine. Just noting the asymmetry for awareness—author's discretion on whether to normalize the single-rule path too.

github-actions · 2026-02-06T16:26:49Z

src/core/concatenate-rules.test.ts

+    expect(result).toBe("# A\n\n# B\n\n# C");
+  });
+
+  it("normalizes CRLF boundaries to one LF blank line", () => {


ℹ️ Info: Good addition of the CRLF test case. Verified that the regex \r?\n correctly strips CRLF at boundaries while preserving CRLF within rule content (e.g., "# A\r\nfoo\r\n" joined with "# B" keeps the internal \r\n intact and only normalizes the boundary).

Jercik

Summary: Found 1 medium issue and 1 info-level observation.

Code review by Copilot (gpt-5.2)

Jercik · 2026-02-06T16:27:50Z

src/core/concatenate-rules.ts

+      /(?:\r?\n)+$/u,
+      "",
+    );
+    const normalizedRuleContent = rule.content.replace(/^(?:\r?\n)+/u, "");


🟡 Medium: The empty-rule check only strips leading newlines (rule.content.replace(/^(?:\r?\n)+/u, "")), so a rule file containing only whitespace (e.g., spaces/tabs + newlines) won’t be treated as empty and may introduce stray whitespace into generated AGENTS.md (which can create unintended Markdown hard-breaks). Consider treating rules as empty when normalizedRuleContent.trim() === "" (or equivalent whitespace-only check) before concatenation.

Jercik · 2026-02-06T16:27:50Z

src/core/concatenate-rules.ts

-    if (combinedContent.endsWith("\n") || rule.content.startsWith("\n")) {
-      combinedContent = `${combinedContent}${rule.content}`;
+    const normalizedCombinedContent = combinedContent.replace(
+      /(?:\r?\n)+$/u,


ℹ️ Info: Good boundary normalization: trimming trailing/leading newline runs with /(?:\r?\n)+$/u and joining with \n\n makes spacing deterministic and also normalizes CRLF input to LF at the join points (covered by the new CRLF test in src/core/concatenate-rules.test.ts).

github-actions · 2026-02-06T16:31:43Z

🎉 This PR is included in version 5.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

fix: enforce one blank line between concatenated rules

9b5cf19

Copilot AI review requested due to automatic review settings February 6, 2026 16:06

Copilot started reviewing on behalf of Jercik February 6, 2026 16:06 View session

This comment was marked as resolved.

Sign in to view

Jercik commented Feb 6, 2026

View reviewed changes

fix: preserve single-rule content in concatenation

45b5cb2

This comment was marked as resolved.

Sign in to view

test: cover middle-rule trailing newline normalization

559874c

This comment was marked as resolved.

Sign in to view

fix: normalize CRLF and skip empty rule boundaries

ff0ce5f

github-actions bot reviewed Feb 6, 2026

View reviewed changes

Jercik commented Feb 6, 2026

View reviewed changes

Jercik merged commit a209f97 into main Feb 6, 2026
8 of 9 checks passed

Jercik deleted the fix/blank-line-between-rules branch February 6, 2026 16:31

github-actions bot added the released label Feb 6, 2026

		@@ -13,12 +14,23 @@ export function concatenateRules(rules: Rule[]): string {
		let combinedContent = firstRule.content;

Conversation

Jercik commented Feb 6, 2026

Summary

Testing

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Jercik left a comment

Choose a reason for hiding this comment

Uh oh!

Jercik Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Jercik left a comment

Choose a reason for hiding this comment

Uh oh!

Jercik Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Jercik Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants