From 4b10db73005f23e108439095081ebe6ddf7632e7 Mon Sep 17 00:00:00 2001 From: Franco Gilio Date: Sun, 6 Sep 2026 14:02:25 +0100 Subject: [PATCH 1/2] fix(native): patch the vendored plugin from a stored stock copy The patch set applied each edit to whatever the vendored file held, so a tree patched by an earlier revision of the script either blocked the run or, when the old anchors still matched, ended up with both revisions' edits. Every patch change needed its own upgrade path. The runner now keeps a copy of each stock target under vendor/nativephp/desktop/.rfa-stock on the first run and patches that copy on every later run, writing the result over the live file. A patched target without a stock copy is refused with a reinstall hint. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01RqwYa86be82YSKkJYb9VLz --- scripts/patch-nativephp.php | 144 +++++++++++++++--- tests/Unit/Scripts/NativePhpPatchSetTest.php | 150 +++++++++++++++++-- 2 files changed, 261 insertions(+), 33 deletions(-) diff --git a/scripts/patch-nativephp.php b/scripts/patch-nativephp.php index 0d091386..f7fa4c4a 100644 --- a/scripts/patch-nativephp.php +++ b/scripts/patch-nativephp.php @@ -1969,38 +1969,54 @@ function rfaNativePhpPatchSet(): array ]; } +/** + * Every edit the set makes carries a `[rfa ]` comment, so a target + * holding this text was written by some revision of this script. + */ +const RFA_PATCH_MARKER = '[rfa '; + /** * Apply the whole patch set under `$distRoot`, or none of it. * + * Patches are always applied to the stock file, never to the output of an + * earlier revision of this script. The first run over a vendor tree copies + * each target into `$stockRoot` before rewriting it; every later run patches + * that copy and writes the result over whatever the target holds, so editing + * a patch needs no upgrade path from its previous output. A target that + * carries rfa edits but has no stock copy comes from a script revision that + * kept none, and is refused with a reinstall hint rather than guessed at. + * * Three phases, in order: * - * 1. **Preflight.** Read each target once and run its edits in memory. A file - * that is missing entirely is reported as absent and skipped — the release - * build re-runs this hook over a pruned `--no-dev` copy where the plugin - * dist legitimately isn't there. A file that is present but whose expected - * shape is gone blocks the run. + * 1. **Preflight.** Read each target once, pick its stock text, and run the + * edits in memory. A file that is missing entirely is reported as absent + * and skipped — the release build re-runs this hook over a pruned + * `--no-dev` copy where the plugin dist legitimately isn't there. A file + * that is present but whose expected shape is gone blocks the run. * 2. **Abort on any block.** Nothing has been written yet, so there is nothing * to undo. - * 3. **Write.** Each changed file goes to a sibling temporary file that is - * renamed into place, so a reader never sees a half-written file. If a - * later write fails, the files already renamed are restored from the + * 3. **Write.** A missing or outdated stock copy is stored first. Each + * changed target then goes to a sibling temporary file that is renamed + * into place, so a reader never sees a half-written file. If a later + * write fails, the targets already renamed are restored from the * originals held in memory. * - * @return array{applied: list, unchanged: list, blocked: list, absent: list, written: list, error: ?string, rolledBack: bool} + * @return array{applied: list, unchanged: list, blocked: list, stale: list, absent: list, written: list, error: ?string, rolledBack: bool} */ -function applyRfaNativePhpPatchSet(string $distRoot): array +function applyRfaNativePhpPatchSet(string $distRoot, string $stockRoot): array { $result = [ 'applied' => [], 'unchanged' => [], 'blocked' => [], + 'stale' => [], 'absent' => [], 'written' => [], 'error' => null, 'rolledBack' => false, ]; - /** @var array $files */ + /** @var array, unchanged: list}> $files */ $files = []; foreach (rfaNativePhpPatchSet() as $patch) { @@ -2013,15 +2029,36 @@ function applyRfaNativePhpPatchSet(string $distRoot): array continue; } - $original = @file_get_contents($path); + $live = @file_get_contents($path); + + if ($live === false) { + $result['blocked'][] = $patch['name']; + + continue; + } + + // A target without rfa edits is stock, and becomes the stock copy + // when the stored one is missing or differs (a reinstalled or + // upgraded plugin). A patched target starts from its stored copy. + $stockPath = $stockRoot.'/'.rfaStockKey($patch['file']); + $stored = is_file($stockPath) ? @file_get_contents($stockPath) : false; + $isPatched = str_contains($live, RFA_PATCH_MARKER); - if ($original === false) { + if ($isPatched && $stored === false) { $result['blocked'][] = $patch['name']; + $result['stale'][] = $patch['name']; continue; } - $files[$path] = ['original' => $original, 'patched' => $original]; + $files[$path] = [ + 'live' => $live, + 'patched' => $isPatched ? $stored : $live, + 'stockPath' => $stockPath, + 'storeStock' => ! $isPatched && $stored !== $live, + 'changed' => [], + 'unchanged' => [], + ]; } $next = $patch['apply']($files[$path]['patched']); @@ -2032,7 +2069,7 @@ function applyRfaNativePhpPatchSet(string $distRoot): array continue; } - $result[$next === $files[$path]['patched'] ? 'unchanged' : 'applied'][] = $patch['name']; + $files[$path][$next === $files[$path]['patched'] ? 'unchanged' : 'changed'][] = $patch['name']; $files[$path]['patched'] = $next; } @@ -2053,7 +2090,23 @@ function applyRfaNativePhpPatchSet(string $distRoot): array $renamed = []; foreach ($files as $path => $contents) { - if ($contents['patched'] === $contents['original']) { + // A target already holding this revision's output reports every patch + // as unchanged, whatever the in-memory run from stock had to do. + if ($contents['patched'] === $contents['live']) { + $result['unchanged'] = [...$result['unchanged'], ...$contents['changed'], ...$contents['unchanged']]; + } else { + $result['applied'] = [...$result['applied'], ...$contents['changed']]; + $result['unchanged'] = [...$result['unchanged'], ...$contents['unchanged']]; + } + + if ($contents['storeStock'] && ! rfaStoreStockCopy($contents['stockPath'], $contents['live'])) { + $result['error'] = $contents['stockPath']; + $result['rolledBack'] = rfaRestoreFiles($renamed); + + return $result; + } + + if ($contents['patched'] === $contents['live']) { continue; } @@ -2064,13 +2117,48 @@ function applyRfaNativePhpPatchSet(string $distRoot): array return $result; } - $renamed[$path] = $contents['original']; + $renamed[$path] = $contents['live']; $result['written'][] = $path; } return $result; } +/** + * Where a target's stock copy lives under the stock root: its path relative + * to the vendored `resources/electron` directory, with `..` folded away. + */ +function rfaStockKey(string $file): string +{ + $segments = []; + + foreach (explode('/', 'electron-plugin/dist/'.$file) as $segment) { + if ($segment === '..') { + array_pop($segments); + + continue; + } + + if ($segment !== '' && $segment !== '.') { + $segments[] = $segment; + } + } + + return implode('/', $segments); +} + +/** + * Store the stock text of a target, creating the directory it lives in. + */ +function rfaStoreStockCopy(string $stockPath, string $contents): bool +{ + if (! is_dir(dirname($stockPath)) && ! @mkdir(dirname($stockPath), 0755, true) && ! is_dir(dirname($stockPath))) { + return false; + } + + return rfaWriteFileAtomically($stockPath, $contents); +} + /** * Replace `$path` with `$contents` through a sibling temporary file, so the * file is either its old self or its new self and never a truncated mix. @@ -2127,9 +2215,19 @@ function rfaNativePhpDistRoot(): string return dirname(__DIR__).'/vendor/nativephp/desktop/resources/electron/electron-plugin/dist'; } +/** + * Where the stock copies of the patched files are kept: inside the vendored + * package, so Composer drops them with the package, and outside its + * `resources/electron` tree, which the build copies whole. + */ +function rfaNativePhpStockRoot(): string +{ + return dirname(__DIR__).'/vendor/nativephp/desktop/.rfa-stock'; +} + // Run when executed directly (not when required by tests) if (realpath($_SERVER['SCRIPT_FILENAME'] ?? '') === realpath(__FILE__)) { - $outcome = applyRfaNativePhpPatchSet(rfaNativePhpDistRoot()); + $outcome = applyRfaNativePhpPatchSet(rfaNativePhpDistRoot(), rfaNativePhpStockRoot()); /** @var array $summaries */ $summaries = array_column(rfaNativePhpPatchSet(), 'summary', 'name'); @@ -2143,6 +2241,16 @@ function rfaNativePhpDistRoot(): string } foreach ($outcome['blocked'] as $name) { + if (in_array($name, $outcome['stale'], true)) { + fwrite(STDERR, sprintf( + " ERROR: the '%s' patch (%s) targets a file already patched by an earlier revision of this script that kept no stock copy, so NOTHING was patched. Reinstall the plugin to start from stock: rm -rf vendor/nativephp/desktop && composer install\n", + $name, + $summaries[$name], + )); + + continue; + } + fwrite(STDERR, sprintf( " ERROR: the '%s' patch (%s) could not be applied, so NOTHING was patched. The vendored NativePHP files changed shape or are incomplete — update scripts/patch-nativephp.php to match them, or reinstall nativephp/desktop.\n", $name, diff --git a/tests/Unit/Scripts/NativePhpPatchSetTest.php b/tests/Unit/Scripts/NativePhpPatchSetTest.php index e7728e1a..bc18b88d 100644 --- a/tests/Unit/Scripts/NativePhpPatchSetTest.php +++ b/tests/Unit/Scripts/NativePhpPatchSetTest.php @@ -1,5 +1,6 @@ , unchanged: list, blocked: list, stale: list, absent: list, written: list, error: ?string, rolledBack: bool} + */ +function applyPatchSet(string $root): array +{ + return applyRfaNativePhpPatchSet($root, stockRootFor($root)); +} + +function stockRootFor(string $root): string +{ + return dirname($root, 2).'/.rfa-stock'; +} + /** @return array */ function distSnapshot(string $root): array { @@ -57,12 +74,20 @@ function distSnapshot(string $root): array ->toEndWith('/vendor/nativephp/desktop/resources/electron/electron-plugin/dist'); }); +test('the stock copies live in the vendored package but outside the copied electron tree', function () { + expect(rfaNativePhpStockRoot()) + ->toEndWith('/vendor/nativephp/desktop/.rfa-stock') + ->and(rfaStockKey('server/php.js'))->toBe('electron-plugin/dist/server/php.js') + ->and(rfaStockKey('../../src/main/index.js'))->toBe('src/main/index.js') + ->and(rfaStockKey('../../electron-builder.mjs'))->toBe('electron-builder.mjs'); +}); + // -- Applying the whole set -- test('applies every patch in one run', function () { $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); expect($outcome['applied'])->toBe(['preload-file-bridge', 'preload-renderer-ready', 'window-theme', 'renderer-ready-window', 'launch-timeline-window', 'server-optimize', 'server-workers', 'launch-timeline-server', 'cookie-after-ready', 'preflight-cache', 'splash-window', 'resolved-appearance', 'early-php-boot', 'launch-timeline', 'php-extraction', 'php-build-wait']) ->and($outcome['blocked'])->toBeEmpty() @@ -119,7 +144,7 @@ function distSnapshot(string $root): array test('a remembered maximize stays transparent until the settled frame is presented', function () { $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); - applyRfaNativePhpPatchSet($root); + applyPatchSet($root); expect(file_get_contents($root.'/server/api/window.js')) ->toContain("opacity: id === 'main' ? 0 : 1") @@ -136,7 +161,7 @@ function distSnapshot(string $root): array // each edit based on the output of the previous edit. $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); - applyRfaNativePhpPatchSet($root); + applyPatchSet($root); $index = file_get_contents($root.'/index.js'); @@ -156,10 +181,10 @@ function distSnapshot(string $root): array test('a second run changes nothing', function () { $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); - applyRfaNativePhpPatchSet($root); + applyPatchSet($root); $afterFirst = distSnapshot($root); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); expect($outcome['applied'])->toBeEmpty() ->and($outcome['unchanged'])->toHaveCount(16) @@ -167,6 +192,99 @@ function distSnapshot(string $root): array ->and(distSnapshot($root))->toBe($afterFirst); }); +// -- Patching from the stock copy -- + +test('every patched target carries the marker the stock check looks for', function () { + $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); + + applyPatchSet($root); + + foreach (distSnapshot($root) as $file => $contents) { + expect($contents)->toContain(RFA_PATCH_MARKER); + } +}); + +test('the first run stores a stock copy of every target', function () { + $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); + $stock = distSnapshot($root); + + applyPatchSet($root); + + foreach ($stock as $file => $contents) { + expect(file_get_contents(stockRootFor($root).'/'.rfaStockKey($file)))->toBe($contents, $file); + } +}); + +test('a tree patched by an earlier revision is re-patched from the stock copy', function () { + // Simulate an older script revision: its output carries rfa edits but not + // this revision's text. No patch anchors on such a file, so the run must + // start over from the stock copy rather than upgrade in place. + $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); + + applyPatchSet($root); + $fresh = distSnapshot($root); + + file_put_contents($root.'/server/php.js', str_replace('rfaNeedsFullOptimize', 'rfaNeedsOptimize', $fresh['server/php.js'])); + file_put_contents($root.'/index.js', "// [rfa patch] an earlier revision\n".stockIndexForSplash()."\n".stockIndex()); + + $outcome = applyPatchSet($root); + + expect($outcome['blocked'])->toBeEmpty() + ->and($outcome['applied'])->toBe(collect(rfaNativePhpPatchSet())->whereIn('file', ['server/php.js', 'index.js'])->pluck('name')->all()) + ->and($outcome['written'])->toBe([$root.'/server/php.js', $root.'/index.js']) + ->and(distSnapshot($root))->toBe($fresh); +}); + +test('a patched tree without a stock copy is refused with a reinstall hint', function () { + $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); + + applyPatchSet($root); + $before = distSnapshot($root); + File::deleteDirectory(stockRootFor($root)); + + $outcome = applyPatchSet($root); + + expect($outcome['stale'])->toBe(collect(rfaNativePhpPatchSet())->pluck('name')->all()) + ->and($outcome['blocked'])->toBe($outcome['stale']) + ->and($outcome['written'])->toBeEmpty() + ->and(distSnapshot($root))->toBe($before); +}); + +test('a reinstalled plugin replaces the stock copy and is patched from the new text', function () { + $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); + + applyPatchSet($root); + + $reinstalled = "// a newer plugin release\n".stockUtils(); + file_put_contents($root.'/server/utils.js', $reinstalled); + + $outcome = applyPatchSet($root); + + expect($outcome['applied'])->toBe(collect(rfaNativePhpPatchSet())->where('file', 'server/utils.js')->pluck('name')->all()) + ->and(file_get_contents(stockRootFor($root).'/electron-plugin/dist/server/utils.js'))->toBe($reinstalled) + ->and(file_get_contents($root.'/server/utils.js'))->toStartWith('// a newer plugin release') + ->and(file_get_contents($root.'/server/utils.js'))->toContain('[rfa cookie after ready]'); +}); + +test('the composer hook names the reinstall command for a stale tree', function () { + $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); + + applyPatchSet($root); + File::deleteDirectory(stockRootFor($root)); + + $process = new Process( + ['php', '-r', sprintf( + 'require %s; $o = applyRfaNativePhpPatchSet(%s, %s); exit($o["blocked"] === [] && $o["error"] === null ? 0 : 1);', + var_export(dirname(__DIR__, 3).'/scripts/patch-nativephp.php', true), + var_export($root, true), + var_export(stockRootFor($root), true), + )], + ); + $process->run(); + + expect($process->getExitCode())->toBe(1); +}); + // -- Preflight: one missing block stops the whole set -- test('one reshaped source block leaves every file untouched', function () { @@ -182,7 +300,7 @@ function distSnapshot(string $root): array }); $before = distSnapshot($root); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); expect($outcome['blocked'])->toBe(['splash-window', 'resolved-appearance', 'early-php-boot']) ->and($outcome['written'])->toBeEmpty() @@ -195,7 +313,7 @@ function distSnapshot(string $root): array }); $before = distSnapshot($root); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); expect($outcome['blocked'])->toBe(['server-optimize', 'server-workers', 'launch-timeline-server']) ->and(distSnapshot($root))->toBe($before); @@ -209,7 +327,7 @@ function distSnapshot(string $root): array chmod($root.'/server/php.js', 0000); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); chmod($root.'/server/php.js', 0644); @@ -227,7 +345,7 @@ function distSnapshot(string $root): array $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_'), fn (string $root) => chmod($root, 0555)); $before = distSnapshot($root); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); chmod($root, 0755); @@ -240,7 +358,7 @@ function distSnapshot(string $root): array test('a run leaves no temporary files behind', function () { $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_')); - applyRfaNativePhpPatchSet($root); + applyPatchSet($root); expect(glob($root.'/*.rfa-patch-*'))->toBeEmpty() ->and(glob($root.'/*/*.rfa-patch-*'))->toBeEmpty(); @@ -249,7 +367,7 @@ function distSnapshot(string $root): array test('a rewritten file keeps its original permissions', function () { $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_'), fn (string $root) => chmod($root.'/server/php.js', 0640)); - applyRfaNativePhpPatchSet($root); + applyPatchSet($root); expect(fileperms($root.'/server/php.js') & 0777)->toBe(0640); }); @@ -259,7 +377,7 @@ function distSnapshot(string $root): array test('an absent dist tree is reported, not failed', function () { // The release build re-runs the hook via `composer install --no-dev` on a // pruned copy where the plugin dist is not present. - $outcome = applyRfaNativePhpPatchSet(sys_get_temp_dir().'/rfa_test_dist_nowhere_'.getmypid()); + $outcome = applyPatchSet(sys_get_temp_dir().'/rfa_test_dist_nowhere_'.getmypid().'/electron-plugin/dist'); expect($outcome['absent'])->toHaveCount(16) ->and($outcome['blocked'])->toBeEmpty() @@ -273,9 +391,10 @@ function distSnapshot(string $root): array $process = new Process( ['php', '-r', sprintf( - 'require %s; $o = applyRfaNativePhpPatchSet(%s); exit($o["blocked"] === [] && $o["error"] === null ? 0 : 1);', + 'require %s; $o = applyRfaNativePhpPatchSet(%s, %s); exit($o["blocked"] === [] && $o["error"] === null ? 0 : 1);', var_export(dirname(__DIR__, 3).'/scripts/patch-nativephp.php', true), var_export($root, true), + var_export(stockRootFor($root), true), )], ); $process->run(); @@ -288,9 +407,10 @@ function distSnapshot(string $root): array $process = new Process( ['php', '-r', sprintf( - 'require %s; $o = applyRfaNativePhpPatchSet(%s); exit($o["blocked"] === [] && $o["error"] === null ? 0 : 1);', + 'require %s; $o = applyRfaNativePhpPatchSet(%s, %s); exit($o["blocked"] === [] && $o["error"] === null ? 0 : 1);', var_export(dirname(__DIR__, 3).'/scripts/patch-nativephp.php', true), var_export($root, true), + var_export(stockRootFor($root), true), )], ); $process->run(); @@ -337,7 +457,7 @@ function distSnapshot(string $root): array $root = stubDistRoot($this->createTempDirectory('rfa_test_dist_'), fn (string $root) => unlink($root.'/server/php.js')); $before = distSnapshot($root); - $outcome = applyRfaNativePhpPatchSet($root); + $outcome = applyPatchSet($root); expect($outcome['absent'])->toBe(['server-optimize', 'server-workers', 'launch-timeline-server']) ->and($outcome['blocked'])->toBe(['server-optimize', 'server-workers', 'launch-timeline-server']) From cc80948b0ea6b86328d26d5204093440c0acce6b Mon Sep 17 00:00:00 2001 From: Franco Gilio Date: Sun, 6 Sep 2026 14:10:42 +0100 Subject: [PATCH 2/2] ci: run checks on pull requests against any base branch Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01RqwYa86be82YSKkJYb9VLz --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d65a74e3..054bd341 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,8 +3,9 @@ name: CI on: push: branches: [main] + # Every pull request, whatever its base: stacked PRs need their checks + # before the branch under them merges. pull_request: - branches: [main] # Cancel superseded PR runs, but never group `push` events together — otherwise # back-to-back merges to main could silently drop intermediate CI runs even