Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix local composite action context data #2710

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented Jul 23, 2023

Action.Id is the correct parent step id, by using ExecutionContext.Id all post steps of Actions are executed with the contextdata of a non direct parent composite action within nested local composite actions.

I open this PR, because issue 2009 has not received any bug fix and I have mentioned this fix in #2009 (comment), since almost a year.

Fixes #2009

Action.Id is the correct parent step id, by using ExecutionContext.Id all post steps of remote Actions are executed with the contextdata of the parent composite action
@ChristopherHX ChristopherHX requested a review from a team as a code owner July 23, 2023 16:06
@ChristopherHX
Copy link
Contributor Author

ChristopherHX commented Jul 25, 2023

To reproduce run: https://github.com/ChristopherHX/local-post-steps-bug

See https://github.com/ChristopherHX/local-post-steps-bug/actions/runs/5661561768/job/15339706765

Post job cleanup.
Post job cleanup.
"{\n  \"depth2\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
Post job cleanup.
"{\n  \"depth2\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
Post job cleanup.
"{\n  \"depth2\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
Post job cleanup.
"{\n  \"depth2\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
  • The first post step executes just fine
  • The second steps context should only contain depth3
  • The third steps context should only contain depth4
  • The fourth steps context should be empty

After applying this change we have

[.github/workflows/main.yml / local-post-steps-bug] Running: Post Run /./depth1
| Post job cleanup.
| Post job cleanup.
| "{\n  \"depth2\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
| Post job cleanup.
| Post job cleanup.
| "{\n  \"depth3\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
| Post job cleanup.
| Post job cleanup.
| "{\n  \"depth4\": {\n    \"outputs\": {},\n    \"outcome\": \"success\",\n    \"conclusion\": \"success\"\n  }\n}"
| Post job cleanup.
| Post job cleanup.
| "{}"
[.github/workflows/main.yml / local-post-steps-bug] Succeeded: Post Run /./depth1

Please Remember to use --disableupdate, if you consider to review this change. Just to make shure you don't fail to review this change due to the update mechanism
Why is the "Update Branch" button gone from PR's of this repo? This makes it a manual task to keep this change uptodate with main I forget that there is now a button on the branch of the PR to update branch

@Sytten
Copy link

Sytten commented Sep 18, 2023

Man this is such a major bug and nobody at github seems to give any sh*t about it. I think its time to complain on hacker news so we get someone to ping internally.

@ChristopherHX
Copy link
Contributor Author

ChristopherHX commented Oct 4, 2023

Yeah seems like the actions/runner team are not even recognizing this as a bug. The last time I opened a PR to this repository, it was an issue including comments which suggested that they have an interest in fixing the bug (still took 1/4 year for them to accept it)

Otherwise these issues would get a "keep" / "ready for dev" label.

One bug issue from me got implemented within two weeks after reporting, that must have been a lot of luck.

Due to this problem beeing very complex we have to hope they revisit it someday. Self-hosted runner users can apply my patch, while it is useless for GitHub Hosted Runner users, which is sad.

For example we are affected, when we want private composite actions from public repos.

If everything is private we can allow access to the private repos and use the repo@ver syntax to avoid hitting this horrible bug

@balintant
Copy link

Bump?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Composite: Step Outputs are not available when in nested composite actions
3 participants