GitHub export: Create new commits in your fork when writing to the upstream repo isn't allowed

[adamziel]

When the GitHub export flow used a forked repository, it would still attempt to create a branch in the upstream repository. This resulted in the following cryptic error:

There was an unexpected error (Not Found), please try again. If the
problem persists, please report it at https://github.com/WordPress/
wordpress-playground/issues.

This PR ensures that branch is created in the push target

Testing instructions

1. Import an upsidedown theme from https://github.com/Automattic/themes
2. Run await playground.writeFile('/wordpress/wp-content/themes/upsidedown/test.txt', 'test'); in devtools
3. Try to export the changes as a PR, confirm a PR got created

Closes #1367

[henriqueiamarino]

@adamziel, when do you believe this is going to be fixed? I just tried, and it's still not working for me.

[adamziel]

@henriqueiamarino it was still deploying. It should be good now – would you confirm? Automattic/themes#7800

[henriqueiamarino]

Yey! It worked well, thanks @adamziel — I just created the following PR:
Automattic/themes#7801

[henriqueiamarino]

I'm not sure if this is important or not, but I'm reporting it anyway.

[adamziel]

Thank you for reporting! There seems to be an issue in that CI flow, it detects way more changes than there are:

I've detected changes to the following themes in this PR: Russell, Pendant, Dawson, Freddie, Course, Professional Business, Montagna, Boxed Bio, Dara, Livro, Winkel, Sunderland, Quadrat, Messagerie, Strand, DOS, Sten, Kingsley, Ames, Maywood, Alves, Mayland (Blocks), Apostrophe 2, Creatio 2, Spearhead (Classic), Masu, Twenty Twenty-Two (Mint), The MENU, Farrow, Loïc, Tomoni, Shhh, Otis, Block Canvas, Independent Publisher 2, Meraki, Common, Illustratr, Overlaid, Blank Canvas, Photos, Elegant Business, Upsidedown, Nested, Jaida, Iotix, Tronar, Epi, Scratchpad, Canard, Exmoor, Shawburn, Poesis, JinJang, Karuna, Lynx, Fewer, Xanadu, Covr, Lineup, Stage, Ueno, Grammer One, Rockfield, Coutoire, Pixl, Brompton, Libre 2, Lettre, Spearhead, BSoJ, Adventurer, Blank Canvas, Stow, Shoreditch, RAW, Radcliffe 2, Issue, Archivo, Pomme, Libretto, Muscat, Stratford, Optimismo, /* Penscratch 2, Intergalactic 2, Artly, Mpho, Luce, Exford, Screenplay, Geologist, Morden, Curriculum, Remote, Cortado, SND, Attar, Mysa, Twenty Twenty-Two (Swiss), Ixion, Texty, Didone, Seedlet, Seedlet (Blocks), Disco, Publication, Mayland, Creatio, Poema, Modern Business, TextBook, Spiel, Arbutus, Alter, Antonia, Rebalance, Entry, Ritratto, Al Dente, Rivington, Negai, Zoologist, Dorna, Kaze, Redhill, Balasana, Bibimbap, Hari, Tenaz, Barnsbury23, Luminance, Barnett, Affinity, Gazette, Paimio, Bennett, Awburn, Calvin, Azur, Bute, Ron, Bibliophile, Verso, Stewart, Appleton, Organizer, Loudness, Sketch, Vivre, Allez, Beep, Calm Business, Hever, StartFit, Assembler, Vetro, Rainfall, Indice, Lativ, Foam, Tú, Heiwa, Programme, Dyad 2, Barnsbury, Blockbase, Jackson, Toujours, Reverie, Ibis, Twenty Twenty-Two (Blue), lois, Trellick, CTLG, Archeo, Dalston, Marl, Lodestar, Calyx, Blogorama, Pieria, Kigen, Hey, Twenty Twenty-Two (Red), Fotograma, Bedrock, Varia, Skatepark, Erma, Blank Canvas, Pique, AltoFocus, George Lois, Storia, Leven, Button 2, Eventual, Ici, Friendly Business.

cc @vcanales – I think that was set up by you? Any change it compares the commit to its HEAD in the same repo, which is likely a stale fork with highly outdated trunk, instead of the target branch in the themes repo?

[brandonpayton]

@adamziel were you able to reproduce this with the themes repo? I'm puzzled about why I was not and would like to learn from this.

[brandonpayton]

@adamziel oops, I also meant to say: Thank you very much for fixing this.

[adamziel]

were you able to reproduce this with the themes repo? I'm puzzled about why I was not and would like to learn from this.

The octokit call I updated failed for me with a 403 error from GitHub. It said something like "your OAuth setup is correct, but the organization limited what OAuth apps can do". Perhaps you have more permissions in the Automattic org than I do and the API call worked for you?

[adamziel]

Actually, I'm having second thoughts about this PR. The export flow will now create a new branch based on the fork's trunk. The fork might be heavily outdated, though, in which case the PR would contain extra changes. I'm not sure why this didn't happen in my test PR. Either way, we should sync the upstream branch to the fork before creating the branch – there's some code to do that in this discussion. I'm worried that could be slow, but slow is better than broken.

Edit: It didn't happen in my PR because the BASE of the Playground-created branch was an old commit that exists in the upstream repo and git is smart enough to know the delta between that BASE and the upstream BASE isn't a part of the PR. Still, sooner or later we'll get a PR that has conflicts the moment it's created so let's pull from upstream to the fork before creating the branch.

[brandonpayton]

Perhaps you have more permissions in the Automattic org than I do and the API call worked for you?

Based on a brief test, I think that is probable.

Still, sooner or later we'll get a PR that has conflicts the moment it's created so let's pull from upstream to the fork before creating the branch.

That sounds good, even if it is slow.