Repository navigation
fix: keep a shorter fence inside a code block - #1227
hbenabdallah wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review.
|
| 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:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- dev-null-sh/dev-null.sh#1210: Both PRs change fenced-code parsing in
CodeSpans.phpandresources/js/lib/code.ts. - dev-null-sh/dev-null.sh#1201: Both PRs change the parser used to preserve Mermaid blocks in
CodeSpans.phpandresources/js/lib/code.ts. - dev-null-sh/dev-null.sh#850: The changed
CodeSpansparsing affects the fenced blocks that PR#850renders in commit share images.
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: 
5
-
Full details: Linked Issues check
-
Explanation
Issue
#603requires a shorter nested fence to remain inside the enclosing code block in the commit preview and posted commit.app/Services/CodeSpans.phpnow captures the opening fence and requires a closing fence with the same length or more.resources/js/lib/code.tsapplies 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. ThesplitCodehelper and theWordmarkTextupdate 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.
|
@xHeaven small fix |
Closes #603
CodeSpans(server) andlib/code.ts(composer preview), so they stay in sync.Summary by CodeRabbit