Skip to content

fix: keep a shorter fence inside a code block - #1227

Open
hbenabdallah wants to merge 1 commit into
dev-null-sh:productionfrom
hbenabdallah:fix/nested-code-fences
Open

hbenabdallah wants to merge 1 commit into
dev-null-sh:productionfrom
hbenabdallah:fix/nested-code-fences

Conversation

@hbenabdallah

@hbenabdallah hbenabdallah commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Closes #603

  • A code block now only closes on a line with at least as many backticks as its opening fence, like CommonMark. A ````` block can hold a ``` fence.
  • Same rule in CodeSpans (server) and lib/code.ts (composer preview), so they stay in sync.
  • Picks up the idea from fix: fix nested code blocks are not split correctly #604, but allows a longer closing fence, which an existing test already expects.
Before After
before after

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of nested Markdown code fences: shorter inner fences remain part of the surrounding code block, while a closing fence of sufficient length ends it.
    • Prose and inline code are now separated correctly around nested fences, including while text is still being typed.
  • Tests
    • Added coverage for nested fences, prose boundaries, and preserving code content during highlighting.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 95b2fbbb-01bf-40ef-803d-c3d9e43d875b
Commits

Reviewing files that changed from the base of the PR and between 028ec16 and b585e34.

Files selected for processing: 5
  • app/Services/CodeSpans.php
  • resources/js/components/wordmark-text.tsx
  • resources/js/lib/code.ts
  • tests/Unit/CodeSpans.test.ts
  • tests/Unit/Services/CodeSpansTest.php

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The PHP and TypeScript code-span parsers now require a closing backtick fence to be at least as long as its opener. The new splitCode helper removes regex capture artifacts, and existing prose-processing functions and WordmarkText use its segments. Tests cover nested fences, following prose, inline code, and highlighting.

Changes: Code Fence Parsing

Layer / File(s) Summary
Fence matching and splitting
app/Services/CodeSpans.php, resources/js/lib/code.ts, tests/Unit/CodeSpans.test.ts, tests/Unit/Services/CodeSpansTest.php
The PHP and TypeScript patterns require closing fences to match or exceed the opening fence length. The splitters separate prose from code matches. Tests cover nested fences and surrounding content.
Adopting splitCode
resources/js/lib/code.ts, resources/js/components/wordmark-text.tsx
Prose-processing functions and WordmarkText use splitCode to obtain prose and code segments.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

Suggested reviewers: mnapoli, xheaven

Merge Risk: Minimal · up to b585e

The nested-fence change is ready to merge after normal checks; no actionable merge risk was identified.

Security architecture risk: Low · up to b585e

The change preserves the existing input-validation, rendering, and outbound-request protections. No introduced security bypass was established. Some compatibility uncertainty remains because previously saved mentions and link previews can differ from newly parsed content.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — User-authored bodies reach the shared parser through preview requests and commit/message processing. Browser reuse includes composer previews and chat fallback rendering. The affected outcomes include displayed blocks, mention interpretation, and automatic link-preview selection, rather than new infrastructure authority.

Security Findings and Attack Paths

  • inferred — No introduced control bypass was established on the inspected paths. Fence reclassification changes which content reaches mention and URL consumers, but does not itself grant a new recipient privilege, bypass notification eligibility, or remove outbound-address checks. Historical metadata differences are compatibility uncertainty, not a verified security finding.

Trust Boundaries and Controls

  • observed — Web and API preview requests reuse body normalization and validation before parsing. The shared rules constrain input type and raw length, and the inspected routes retain their existing authentication requirements.
  • observed — Browser-generated block HTML still passes through escapeHtml. Server blocks still use the existing highlighter, whose documented contract includes escaping. The existing renderer consumes block HTML through CodeBlock; this PR changes segmentation, not that HTML trust contract.
  • observed — Outbound preview fetching remains independently constrained to HTTP(S), allowed ports, URLs without credentials, and public resolved addresses. Redirects are rechecked, connections are pinned to checked addresses, and requests retain time and download limits.

Resilience and Maintainability Implications

  • observed — The existing creation transactions, sender/client-id message retry handling, notification deduplication, and preview-job terminal handling are unchanged. The parser change supplies different derived values without introducing a new persistence or recovery protocol.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: shorter nested fences remain inside an enclosing code block.
Linked Issues check Passed Issue #603 requires a shorter nested fence to remain inside the enclosing code block in the commit preview and posted commit. app/Services/CodeSpans.php now captures the opening fence and requires a…
Out of Scope Changes check Passed The changes stay within issue #603. The splitCode helper and the WordmarkText update preserve correct segmentation after the regex capture change. The added PHP and TypeScript tests verify the req…
Docstring Coverage Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviv…
Full details: Linked Issues check

Explanation

Issue #603 requires a shorter nested fence to remain inside the enclosing code block in the commit preview and posted commit. app/Services/CodeSpans.php now captures the opening fence and requires a closing fence with the same length or more. resources/js/lib/code.ts applies the same rule and preserves prose/code segmentation. Tests cover nested fences, longer closing fences, committed input, and typed input in both server and composer code.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #603. The splitCode helper and the WordmarkText update preserve correct segmentation after the regex capture change. The added PHP and TypeScript tests verify the requested behavior. No unrelated product behavior is changed.

Full details: Docstring Coverage

Explanation

Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 8 functions across 5 files.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@hbenabdallah

Copy link
Copy Markdown
Contributor Author

@xHeaven small fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

End of code block fence is incorrect when fences are nested

1 participant