From bec68a5fce8e526ead513a76e5d95421d7bb2f9b Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 15 Sep 2026 13:33:52 -0500 Subject: [PATCH 1/4] Trac PRs: Convert Markdown headings to Trac headings. Trac reads WikiCreole's `=` rather than Markdown's `#`, so every heading in a pull request description has been reaching the ticket as its own literal text. The comment composer now writes a Trac heading of the same depth, and escapes a line-opening `=` in the body first so that only the markup it writes is markup. Headings are converted per span of the fence split, which begins and ends where a line does, so a heading holding inline code stays one heading and a `#` inside a fenced block stays code. Co-Authored-By: Claude Opus 5 (1M context) --- .../public_html/dotorg/trac/pr/functions.php | 36 ++- .../pr/tests/Trac_Comment_Heading_Test.php | 304 ++++++++++++++++++ 2 files changed, 339 insertions(+), 1 deletion(-) create mode 100644 api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php index a2003c0aa4..4c52291c5d 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php @@ -497,6 +497,32 @@ function trac_comment_code_block( $fence ) { $m['indent'] . '}}}'; } +/** + * Converts the ATX headings of one span to Trac headings. + * + * @param string $text A span of the pull request lying outside any fenced code block. + * @return string|false The span with its headings converted, or false if it cannot be built. + */ +function trac_comment_headings( $text ) { + $text = preg_replace( '~^[ \t>]*\K(?=={1,6}[ \t])~m', '!', $text ); + if ( null === $text ) { + return false; + } + + $text = preg_replace_callback( + '~^(?P[ \t]*>[ \t>]*)?[ \t]{0,3}(?P\#{1,6})[ \t]+(?P.+?)(?:[ \t]+\#+)?[ \t]*$~m', + function ( $m ) { + $level = str_repeat( '=', strlen( $m['hashes'] ) ); + $cite = '' === $m['cite'] ? '' : rtrim( $m['cite'] ) . ' '; + + return "{$cite}{$level} {$m['text']} {$level}"; + }, + $text + ); + + return is_string( $text ) ? $text : false; +} + /** * Converts one span of pull request text to Trac wiki markup. * @@ -642,6 +668,7 @@ function ( $m ) { * This: * - Strips HTML comments * - Converts code blocks + * - Converts headings * - Converts image embeds * - Converts links * - Converts tables @@ -666,7 +693,14 @@ function format_github_content_for_trac_comment( $desc ) { } foreach ( $parts as $i => $part ) { - $part = ( $i % 2 ) ? trac_comment_code_block( $part ) : trac_comment_wiki_text( $part ); + if ( $i % 2 ) { + $part = trac_comment_code_block( $part ); + } else { + // A heading is a whole line, and only this split's spans begin and end where lines do. + $part = trac_comment_headings( $part ); + $part = is_string( $part ) ? trac_comment_wiki_text( $part ) : false; + } + if ( false === $part ) { return false; } diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php new file mode 100644 index 0000000000..378d1f6550 --- /dev/null +++ b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php @@ -0,0 +1,304 @@ +assertIsString( $desc ); + $this->assertStringContainsString( $expect, $desc ); + } + + /** + * Supplies one pull request heading per level. + * + * @return array Bodies and the headings they must compose. + */ + public function data_heading_levels(): array { + return array( + 'level one' => array( "# Summary\n\nText.\n", '= Summary =' ), + 'level two' => array( "## Summary\n\nText.\n", '== Summary ==' ), + 'level three' => array( "### Summary\n\nText.\n", '=== Summary ===' ), + 'level four' => array( "#### Summary\n\nText.\n", '==== Summary ====' ), + 'level five' => array( "##### Summary\n\nText.\n", '===== Summary =====' ), + 'level six' => array( "###### Summary\n\nText.\n", '====== Summary ======' ), + ); + } + + /** + * A seventh `#` is past Trac's deepest heading, so the line is left as text. + * + * @return void + */ + public function test_a_seventh_level_is_not_a_heading() { + $desc = format_github_content_for_trac_comment( "####### Summary\n\nText.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '####### Summary', $desc ); + } + + /** + * Markdown's optional closing run belongs to the marker, not to the title. + * + * @return void + */ + public function test_a_closed_heading_keeps_only_its_title() { + $desc = format_github_content_for_trac_comment( "## Summary ##\n\nText.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '== Summary ==', $desc ); + $this->assertStringNotContainsString( '## ', $desc ); + } + + /** + * Up to three leading spaces are Markdown's, and Trac has no use for them. + * + * @return void + */ + public function test_an_indented_heading_is_converted() { + $desc = format_github_content_for_trac_comment( "Text.\n\n ## Summary\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( "\n== Summary ==", $desc ); + } + + /** + * A line that only looks like a heading must reach Trac as it was written. + * + * @dataProvider data_non_headings + * + * @param string $body A pull request body opening a line with `#`. + * @return void + */ + public function test_a_hash_that_is_not_a_heading_is_left_alone( string $body ) { + $this->assertSame( trim( $body ), format_github_content_for_trac_comment( $body ) ); + } + + /** + * Supplies the line-opening hashes that are not Markdown headings. + * + * @return array Bodies that must be composed unchanged. + */ + public function data_non_headings(): array { + return array( + 'ticket reference' => array( "#65845 is the ticket.\n" ), + 'no space' => array( "#Summary\n" ), + 'interpreter line' => array( "#!/bin/sh\n" ), + 'empty heading' => array( "## \n" ), + 'four spaces' => array( "Text.\n\n ## Summary\n" ), + ); + } + + /** + * Trac strips a citation's `>` before it reads the line, so a quote has headings too. + * + * @dataProvider data_cited_headings + * + * @param string $body A pull request body quoting a heading. + * @param string $expect The quoted line it must compose. + * @return void + */ + public function test_a_cited_heading_keeps_its_citation( string $body, string $expect ) { + $desc = format_github_content_for_trac_comment( $body ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( $expect, $desc ); + } + + /** + * Supplies the quoted lines Trac reads as a heading. + * + * @return array Bodies and the quoted lines they must compose. + */ + public function data_cited_headings(): array { + return array( + 'converted' => array( "> ## Summary\n", '> == Summary ==' ), + 'no space' => array( ">## Summary\n", '> == Summary ==' ), + 'nested' => array( "> > ## Summary\n", '> > == Summary ==' ), + 'escaped' => array( "> == Summary\n", '> !== Summary' ), + 'escaped nested' => array( "> > == Summary\n", '> > !== Summary' ), + 'escaped indented' => array( "> == Summary\n", '> !== Summary' ), + ); + } + + /** + * A quotation deep enough to exhaust PCRE's stack must still reach the ticket. + * + * A pattern that repeats a group rather than naming a character class gives up on + * a line like this, and a refused span takes the whole comment with it. + * + * @dataProvider data_deep_citations + * + * @param string $line The line to quote, once the citation is prepended. + * @param string $expect The end of the line it must compose. + * @return void + */ + public function test_a_deep_citation_does_not_refuse_the_comment( string $line, string $expect ) { + // Two characters per level, within GitHub's 65536 character body. + $desc = format_github_content_for_trac_comment( str_repeat( '> ', 30000 ) . $line ); + + $this->assertIsString( $desc ); + $this->assertStringEndsWith( $expect, $desc ); + } + + /** + * Supplies the deeply quoted lines that must survive the composer. + * + * @return array Lines to quote and the endings they must compose. + */ + public function data_deep_citations(): array { + return array( + 'converted' => array( "## Summary\n", '== Summary ==' ), + 'escaped' => array( "== Summary\n", '!== Summary' ), + 'neither' => array( "Summary\n", '> Summary' ), + ); + } + + /** + * A heading's own inline code must stay inside the heading. + * + * The composer splits a span on its inline code, so a heading cut in half by a + * backtick would otherwise close over the first piece alone. + * + * @return void + */ + public function test_a_heading_holding_inline_code_stays_one_heading() { + $desc = format_github_content_for_trac_comment( "## The `retention-days` key\n\nText.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '== The `retention-days` key ==', $desc ); + } + + /** + * Inline code that opens a line is content, not a marker. + * + * @return void + */ + public function test_a_hash_inside_inline_code_is_not_a_heading() { + $desc = format_github_content_for_trac_comment( "Write `## Summary` for a heading.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '`## Summary`', $desc ); + } + + /** + * A heading's links and images are converted with the rest of the prose. + * + * @return void + */ + public function test_a_heading_still_converts_its_links() { + $desc = format_github_content_for_trac_comment( "## See [the docs](https://example.org/doc)\n" ); + + $this->assertSame( '== See [https://example.org/doc the docs] ==', $desc ); + } + + /** + * A fenced block's contents are code, so its hashes are not headings. + * + * @return void + */ + public function test_a_hash_inside_a_fence_is_not_a_heading() { + $desc = format_github_content_for_trac_comment( "Text.\n\n```sh\n## Summary\n```\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( "\n## Summary\n", $desc ); + $this->assertStringNotContainsString( '== Summary ==', $desc ); + } + + /** + * Trac's own heading marker is the composer's to write, not the body's. + * + * @dataProvider data_wiki_headings + * + * @param string $body A pull request body opening a line with `=`. + * @param string $expect The escaped line it must compose. + * @return void + */ + public function test_a_wiki_heading_in_the_body_is_escaped( string $body, string $expect ) { + $desc = format_github_content_for_trac_comment( $body ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( $expect, $desc ); + } + + /** + * Supplies the line-opening `=` runs Trac would read as a heading. + * + * @return array Bodies and the escaped lines they must compose. + */ + public function data_wiki_headings(): array { + return array( + 'bare' => array( "== Summary\n", '!== Summary' ), + 'closed' => array( "== Summary ==\n", '!== Summary ==' ), + 'indented' => array( "Text.\n\n = Summary =\n", "\n !=" ), + 'deepest' => array( "====== Summary\n", '!====== Summary' ), + 'tab spaced' => array( "==\tSummary\n", "!==\tSummary" ), + ); + } + + /** + * An `=` that opens no heading of Trac's is prose, and stays prose. + * + * @dataProvider data_inert_equals + * + * @param string $body A pull request body opening a line with `=`. + * @return void + */ + public function test_an_inert_equals_is_left_alone( string $body ) { + $this->assertSame( trim( $body ), format_github_content_for_trac_comment( $body ) ); + } + + /** + * Supplies the line-opening `=` runs Trac reads as text. + * + * @return array Bodies that must be composed unchanged. + */ + public function data_inert_equals(): array { + return array( + 'no space' => array( "=Summary\n" ), + 'too deep' => array( "======= Summary\n" ), + 'arrow' => array( "=> Returns the value.\n" ), + ); + } + + /** + * A heading must not gain the escape written for the line above it. + * + * @return void + */ + public function test_an_escape_and_a_heading_can_share_a_body() { + $desc = format_github_content_for_trac_comment( "== Written\n\n## Composed\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '!== Written', $desc ); + $this->assertStringContainsString( '== Composed ==', $desc ); + $this->assertStringNotContainsString( '!== Composed', $desc ); + } +} From 6525e3a23576bd9e3c2ac4aeac4a5a076e18f848 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 15 Sep 2026 13:34:03 -0500 Subject: [PATCH 2/4] Trac PRs: Stop a pull request being mentioned on a ticket twice. GitHub delivers several `pull_request` events for one pull request at once, and the webhook read `trac_github_prs` for an existing ticket reference before inserting one. Two deliveries a second apart both read before either wrote, so both went on to comment on the ticket. Only one row was ever added: the `trac_2` unique key over `trac`, `ticket`, `repo` and `pr` refused the second. Its insert returned false and nothing looked, so the comment was posted anyway. Reading that result is the whole fix, and the unique key is what makes it exact however the two deliveries interleave. The read is kept. It cannot be relied on to tell the deliveries apart, but it keeps every later event for a recorded pull request from attempting an insert that the unique key would only refuse. Its ticket comparison is made numeric for the same reason: the column is an integer, and a strict comparison against the string the reference was parsed from would miss every time were it ever read back as one. Co-Authored-By: Claude Opus 5 (1M context) --- .../public_html/dotorg/trac/pr/webhook.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php b/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php index 7e705b9709..fe0da1fe8c 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php @@ -73,7 +73,7 @@ function verify_signature() { foreach ( $existing_refs as $ref ) { if ( $ref->trac === $pr_data->trac_ticket[0] && - $ref->ticket === $pr_data->trac_ticket[1] + (int) $ref->ticket === (int) $pr_data->trac_ticket[1] ) { $matched_existing_ref = true; } @@ -85,11 +85,14 @@ function verify_signature() { unset( $_pr_data_no_ticket->trac_ticket, $_pr_data_no_ticket->body ); // Step 3. If not in DB, or $pr_data->trac_ticket isn't yet in the DB, add a new row of it. + $user_id = 0; + $new_ref = false; + if ( $pr_data->trac_ticket && ( ! $existing_refs || ! $matched_existing_ref ) ) { $user_id = (int) find_wporg_user_by_github( $pr_data->user->name, 'ID' ); - $wpdb->insert( + $new_ref = (bool) $wpdb->insert( 'trac_github_prs', [ 'created' => gmdate( 'Y-m-d H:i:s', strtotime( $pr_data->created_at ) ), @@ -102,6 +105,10 @@ function verify_signature() { 'author' => $user_id, ] ); + } + + // Only the request whose row was added mentions the PR on the ticket. + if ( $new_ref ) { // Add a mention to the Trac Ticket. $trac = get_trac_instance( $pr_data->trac_ticket[0] ); From 5f4cdc38dc68bee9039f2e63b7a0c8286f18cf2b Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 15 Sep 2026 15:52:23 -0500 Subject: [PATCH 3/4] Trac PRs: Escape a Trac heading behind any whitespace Trac skips The escape written for a heading marker only stepped over ASCII spaces, tabs and a citation's `>`, but Trac reads a line with Python's whitespace class, which is wider. A pull request body could open a line with a no-break space and have the `=` behind it render as a heading of its own. `trac_comment_skipped()` already names that class for the two patterns either side of this one, so it names it here too. Co-Authored-By: Claude Opus 5 (1M context) --- .../public_html/dotorg/trac/pr/functions.php | 4 +++- .../trac/pr/tests/Trac_Comment_Heading_Test.php | 15 ++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php index 4c52291c5d..a7cc475f8c 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php @@ -504,7 +504,9 @@ function trac_comment_code_block( $fence ) { * @return string|false The span with its headings converted, or false if it cannot be built. */ function trac_comment_headings( $text ) { - $text = preg_replace( '~^[ \t>]*\K(?=={1,6}[ \t])~m', '!', $text ); + $skipped = trac_comment_skipped(); + + $text = preg_replace( "~^{$skipped}*\K(?=={1,6}[ \t])~mu", '!', $text ); if ( null === $text ) { return false; } diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php index 378d1f6550..cac77818d4 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php @@ -255,11 +255,16 @@ public function test_a_wiki_heading_in_the_body_is_escaped( string $body, string */ public function data_wiki_headings(): array { return array( - 'bare' => array( "== Summary\n", '!== Summary' ), - 'closed' => array( "== Summary ==\n", '!== Summary ==' ), - 'indented' => array( "Text.\n\n = Summary =\n", "\n !=" ), - 'deepest' => array( "====== Summary\n", '!====== Summary' ), - 'tab spaced' => array( "==\tSummary\n", "!==\tSummary" ), + 'bare' => array( "== Summary\n", '!== Summary' ), + 'closed' => array( "== Summary ==\n", '!== Summary ==' ), + 'indented' => array( "Text.\n\n = Summary =\n", "\n !=" ), + 'deepest' => array( "====== Summary\n", '!====== Summary' ), + 'tab spaced' => array( "==\tSummary\n", "!==\tSummary" ), + // Trac steps over Python's whitespace, which is wider than PCRE's. + 'no-break' => array( "Text.\n\n\u{00A0}== Summary\n", "\u{00A0}!== Summary" ), + 'en space' => array( "Text.\n\n\u{2002}== Summary\n", "\u{2002}!== Summary" ), + 'ideographic' => array( "Text.\n\n\u{3000}== Summary\n", "\u{3000}!== Summary" ), + 'unit separator' => array( "Text.\n\n\x1f== Summary\n", "\x1f!== Summary" ), ); } From 66d864a8778bf36b0efef8595557ecc3e5c86546 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 15 Sep 2026 15:59:38 -0500 Subject: [PATCH 4/4] Trac PRs: Escape a Trac heading separated by any whitespace Trac skips The previous commit widened the whitespace a heading marker may hide behind but left the whitespace that separates the marker from its title as ASCII, so a `=` run followed by a no-break space still reached Trac as a heading. The citation marker is dropped from that second class: Trac separates a heading with whitespace alone, so a `>` after the `=` run is text, and escaping it would put a literal `!` in front of an arrow. Co-Authored-By: Claude Opus 5 (1M context) --- api.wordpress.org/public_html/dotorg/trac/pr/functions.php | 5 ++++- .../dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php index a7cc475f8c..78924963d6 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php @@ -506,7 +506,10 @@ function trac_comment_code_block( $fence ) { function trac_comment_headings( $text ) { $skipped = trac_comment_skipped(); - $text = preg_replace( "~^{$skipped}*\K(?=={1,6}[ \t])~mu", '!', $text ); + // A heading is separated by whitespace alone; a `>` after its `=` run is text. + $space = str_replace( '>', '', $skipped ); + + $text = preg_replace( "~^{$skipped}*\K(?=={1,6}{$space})~mu", '!', $text ); if ( null === $text ) { return false; } diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php index cac77818d4..1b781653d6 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php @@ -265,6 +265,10 @@ public function data_wiki_headings(): array { 'en space' => array( "Text.\n\n\u{2002}== Summary\n", "\u{2002}!== Summary" ), 'ideographic' => array( "Text.\n\n\u{3000}== Summary\n", "\u{3000}!== Summary" ), 'unit separator' => array( "Text.\n\n\x1f== Summary\n", "\x1f!== Summary" ), + // The same class again, this time separating the marker from the title. + 'no-break sep' => array( "Text.\n\n==\u{00A0}Summary\n", "!==\u{00A0}Summary" ), + 'en space sep' => array( "Text.\n\n==\u{2002}Summary\n", "!==\u{2002}Summary" ), + 'cited sep' => array( "Text.\n\n> ==\u{00A0}Summary\n", "> !==\u{00A0}Summary" ), ); } @@ -288,6 +292,7 @@ public function test_an_inert_equals_is_left_alone( string $body ) { public function data_inert_equals(): array { return array( 'no space' => array( "=Summary\n" ), + 'citation' => array( "==> Not a heading.\n" ), 'too deep' => array( "======= Summary\n" ), 'arrow' => array( "=> Returns the value.\n" ), );