Create PR from fork by webwarrior-ws · Pull Request #1 · tarsgate/pi-coding-agent-action

webwarrior-ws · 2026-09-09T13:09:41Z

If there are no permissions to push to the repository in question.

knocte · 2026-09-09T13:17:46Z

CI is broken

knocte · 2026-09-10T08:51:12Z

CI is still broken. @webwarrior-ws do you have github email notifications about this enabled?

webwarrior-ws · 2026-09-10T08:58:42Z

CI is still broken. @webwarrior-ws do you have github email notifications about this enabled?

I've disabled notifications for CI failures because it's just noise in most cases.

But I've seen that CI is still broken. It fails on "Upload coverage to Codecov". Perhaps authentication issue? I don't know what this Codecov service is.

knocte · 2026-09-10T09:02:05Z

ok I thought it was still complaining about prettier format

knocte · 2026-09-10T09:02:37Z

But I've seen that CI is still broken. It fails on "Upload coverage to Codecov".

Let's create a PR upstream that changes that step to not be executed in forks.

knocte · 2026-09-10T09:04:09Z

ok I thought it was still complaining about prettier format

So can conventions' PR be merged now?

webwarrior-ws · 2026-09-10T09:06:37Z

ok I thought it was still complaining about prettier format

So can conventions' PR be merged now?

Yes

knocte · 2026-09-10T09:10:58Z

Ok done, but please rebase before testing

knocte · 2026-09-10T09:57:44Z

It seems the experiment worked, so let's squash these commits and after that include the commit from PR2 as 1st commit of this one, to get green CI.

webwarrior-ws · 2026-09-10T10:07:31Z

It seems the experiment worked, so let's squash these commits and after that include the commit from PR2 as 1st commit of this one, to get green CI.

Done.

knocte · 2026-09-10T10:42:49Z

This mirrors how human contributors work.

Let's add to this: "(especially in an environment where the repo to work with is public/open but direct commit access is not, while PRs are accepted from any github user account, regardless of the permissions of the latter)".

After that, you can propose new PR upstream, with that single commit (not 2 commits).

knocte · 2026-09-10T12:08:41Z

@webwarrior-ws Given that the maintainer merged already your first PR, you can actually remove that commit from here, rebase, and then propose the same commit that ends up here (same commit hash) in the new PR for upstream. Let's do this before or at EOD time, not Monday. Cheers

knocte · 2026-09-17T08:04:54Z

@webwarrior-ws Given that the maintainer merged already your first PR, you can actually remove that commit from here,

Why was this not done? I still see the commit "GithubCI: don't run "Upload coverage to Codecov" step in forks" here

If there are insufficient rights to push to the repository, agent will open a PR from a fork. This mirrors how human contributors work (especially in an environment where the repo to work with is public/open but direct commit access is not, while PRs are accepted from any github user account, regardless of the permissions of the latter).

knocte · 2026-09-17T09:33:05Z

Missing commit that updates dist/ ?

github-actions · 2026-09-17T09:36:24Z

Fallow combined report

Quality gate passed

No findings for this pull request.

Generated by fallow.

knocte · 2026-09-17T09:39:30Z

Upstream's commits are never titled "Build bundle" for dist/ updates.

And added tests for new functionality.

webwarrior-ws · 2026-09-17T10:13:57Z

Should I update shaftoe#411 to this PR's branch?

knocte · 2026-09-17T10:27:34Z

What do you think I'm going to reply to that?

webwarrior-ws · 2026-09-17T10:29:42Z

What do you think I'm going to reply to that?

If I had to guess, I'd guess "yes". But I'd rather get an actual answer.

knocte · 2026-09-17T10:31:15Z

You're wrong, I would reply: did you test this? And of course what I would hoping is that with this question you understand that I'm asking if you tested all scenarios: when it should create a fork when fork doesn't exist, when it should not create a fork because it already exists, and when it should not create a fork because pushing directly works.

webwarrior-ws · 2026-09-17T10:38:18Z

You're wrong, I would reply: did you test this? And of course what I would hoping is that with this question you understand that I'm asking if you tested all scenarios: when it should create a fork when fork doesn't exist, when it should not create a fork because it already exists, and when it should not create a fork because pushing directly works.

The second scenario was already tested.

For the first one, I can ask pi to create a PR in repo where nodefect doesn't have a fork.

But the third one I can't really test. Not for a situation where it's not nodefect's own repo.

knocte · 2026-09-17T10:43:31Z

For the first one, I can ask pi to create a PR in repo where nodefect doesn't have a fork.

Why did you start testing this already? Please stop making me hand-hold you all the time, I told you you're a software developer, not a programmer.

But the third one I can't really test. Not for a situation where it's not nodefect's own repo.

Bullshit excuses, you can test it perfectly. If you need X, Y, Z in order to do it, you can ask me, instead of saying that you can't.

knocte · 2026-09-17T10:48:35Z

BTW I REMIND YOU THAT CONVENTIONS IS POINTING AT PR3 NOW, NOT THIS PR. THEREFORE ANY TESTING YOU'RE DOING IS NOT AGAINST THIS CODE. (AND I CHECKED THAT BOTH BRANCHES DIFFER BEFORE POSTING THIS COMMENT.)

webwarrior-ws · 2026-09-17T10:50:46Z

BTW I REMIND YOU THAT CONVENTIONS IS POINTING AT PR3 NOW, NOT THIS PR. THEREFORE ANY TESTING YOU'RE DOING IS NOT AGAINST THIS CODE. (AND I CHECKED THAT BOTH BRANCHES DIFFER BEFORE POSTING THIS COMMENT.)

They differ only in some debug output redirection.

knocte · 2026-09-17T10:55:05Z

I don't care? I only care about commit hashes. If commit hashes are not the same, you're not testing the same thing. And same argument I will have whenever you intend to push this work to upstream PR later: I will compare commit hashes, and if they are not the same, I will not consider the work done.

knocte · 2026-09-17T10:57:20Z

BTW upstream's PR's commit hash is f0d2, why the hell do we have a different commit hash here? fix that first before testing anything!!!!!

webwarrior-ws · 2026-09-17T10:59:33Z

BTW upstream's PR's commit hash is f0d2, why the hell do we have a different commit hash here? fix that first before testing anything!!!!!

Because these PRs are from different branches. I asked earlier if I should update upstream PR to this and you said to test first.

knocte · 2026-09-17T11:01:48Z

I'M TALKING ABOUT THE 1ST COMMIT OF THIS PR!!!! NOT THE REST!!!!

knocte · 2026-09-17T11:02:21Z

I KNOW THAT THIS PR HAS MORE COMMITS THAN UPSTREAM PR, I KNOW HOW TO COUNT!!!

webwarrior-ws · 2026-09-17T11:03:48Z

This PR was rebased. The upstream PR wasn't.

knocte · 2026-09-17T11:05:32Z

YOU CREATED THE UPSTREAM PR AFTER YOUR 1ST PR WAS MERGED!!! THEREFORE THERE WAS NO NEED TO REBASE THIS

knocte · 2026-09-17T11:08:13Z

FFS! even tho you created the 2nd upstream PR after your 1st was merged, you didn't rebase it before creating it!!!

webwarrior-ws · 2026-09-17T11:08:49Z

@webwarrior-ws Given that the maintainer merged already your first PR, you can actually remove that commit from here,

Why was this not done? I still see the commit "GithubCI: don't run "Upload coverage to Codecov" step in forks" here

YOU CREATED THE UPSTREAM PR AFTER YOUR 1ST PR WAS MERGED!!! THEREFORE THERE WAS NO NEED TO REBASE THIS

I rebased to get rid of "GithubCI: don't run "Upload coverage to Codecov" step in forks" commit.

webwarrior-ws · 2026-09-17T11:09:39Z

FFS! even tho you created the 2nd upstream PR after your 1st was merged, you didn't rebase it before creating it!!!

There was no need to.

knocte · 2026-09-17T11:09:50Z

I rebased to get rid of "GithubCI: don't run "Upload coverage to Codecov" step in forks" commit.

YOU SHOULD HAVE REBASED BEFORE CREATING THE 2ND UPSTREAM PR FFS! THIS WAY YOU WOULDNT HAVE NEEDED TO REBASE THIS REPO!!!!!

knocte · 2026-09-17T11:10:24Z

Sighhhhhhhhhhhh, so now we need to force-push that upstream PR in order to update it, FFS

knocte · 2026-09-17T11:11:18Z

Sighhhhhhhhhhhh, so now we need to force-push that upstream PR in order to update it, FFS

You're allowed to do the above, now that I realise that there's no other way. BUT DO NOT UPDATE UPSTREAM PR UNTIL YOU HAVE TESTED ALL 3 SCENARIOS!!!!! AND I WANT ALL COMMIT HASHES TO MATCH!!!!

webwarrior-ws · 2026-09-17T11:12:56Z

I tried to test the first scenario: tarsgate/conventions#369

And it failed, but not because there's a bug in tools, but because the agent tried to open a PR against skynot from conventions repo.

So I don't know what to make of it.

knocte · 2026-09-17T11:17:14Z

So I don't know what to make of it.

What happens there is that you're not testing what I told you to test, you're testing a 4th scenario that we didn't talk about: when bot needs to create a PR on a repo that is different than where the workflow is running (and by different I mean completely different: we're not talking about a fork-or-not-fork difference, it's not conventions!). YOU HAVE TO TEST THOSE 3 SCENARIOS WITH THE CONVENTIONS REPO BECAUSE IT IS IN THE CONVENTIONS REPO WHERE THE BOT'S WORKFLOW IS!!!!

webwarrior-ws · 2026-09-17T11:22:00Z

Then I need:

https://github.com/nodefect/conventions fork to be deleted for testing scenario 1
nodefect given push permission to https://github.com/tarsgate/conventions repo for testing scenario 3

knocte · 2026-09-17T11:24:33Z

Then I need:

Those things are better that you ask me in the channel so that I can do FASTER than acting on an email.

knocte · 2026-09-17T11:25:24Z

https://github.com/nodefect/conventions fork to be deleted for testing scenario 1

Better to test scenario 2 before you ask me for that, no? This way you don't need to wait for me and can start working already.

webwarrior-ws · 2026-09-17T11:26:37Z

https://github.com/nodefect/conventions fork to be deleted for testing scenario 1

Better to test scenario 2 before you ask me for that, no? This way you don't need to wait for me and can start working already.

It was already tested. Or do you insist it to be re-tested with this particular branch?

webwarrior-ws · 2026-09-17T11:37:50Z

Testing scenario 2: tarsgate/conventions#370

knocte · 2026-09-17T11:41:55Z

Or do you insist it to be re-tested with this particular branch?

COMMIT HASHES, DO THEY MATCH??? DONT TALK TO ME ABOUT BRANCHES!!!!!

webwarrior-ws marked this pull request as draft September 9, 2026 13:09

knocte reviewed Sep 9, 2026

View reviewed changes

Comment thread packages/pi-orchestrator/src/pi/prompt.ts Outdated

knocte reviewed Sep 9, 2026

View reviewed changes

Comment thread packages/pi-orchestrator/src/pi/prompt.ts Outdated

knocte reviewed Sep 9, 2026

View reviewed changes

Comment thread action.yml

knocte changed the title ~~Create pr from fork~~ Create PR from fork Sep 9, 2026

webwarrior-ws force-pushed the create-pr-from-fork branch from 5a98c87 to 82e1ec0 Compare September 10, 2026 08:32

webwarrior-ws force-pushed the create-pr-from-fork branch from 82e1ec0 to c738b24 Compare September 10, 2026 09:14

webwarrior-ws force-pushed the create-pr-from-fork branch from 92a9a7e to 6202e1e Compare September 10, 2026 10:04

knocte reviewed Sep 10, 2026

View reviewed changes

Comment thread packages/pi-orchestrator/src/pi/prompt.ts Outdated

webwarrior-ws force-pushed the create-pr-from-fork branch 2 times, most recently from a8c21b8 to 52bf206 Compare September 10, 2026 10:24

webwarrior-ws force-pushed the create-pr-from-fork branch from 52bf206 to b07ed47 Compare September 17, 2026 08:11

Implement tools support for creating PR in a fork

68ea3ba

And added tests for new functionality.

webwarrior-ws force-pushed the create-pr-from-fork branch from 2597938 to 68ea3ba Compare September 17, 2026 10:02

Conversation

webwarrior-ws commented Sep 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knocte commented Sep 9, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

webwarrior-ws commented Sep 10, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

webwarrior-ws commented Sep 10, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

webwarrior-ws commented Sep 10, 2026

Uh oh!

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

knocte commented Sep 10, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fallow combined report

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

webwarrior-ws commented Sep 17, 2026

Uh oh!

knocte commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026 •

edited

Loading

webwarrior-ws commented Sep 17, 2026 •

edited

Loading