fix: Add Cmd Default/Input support to cleanup by tkalus · Pull Request #183 · webfactory/ssh-agent · GitHub

Bring post action step into consistency with main for changes introduced in #154.

Without this change, sshAgentCmd is undefined when passed to execFileSync() during cleanup and post is never successful.

Post job cleanup consistently showing:

Stopping SSH agent
The "file" argument must be of type string. Received undefined
Error stopping the SSH agent, proceeding anyway

Also add entries to the CHANGELOG, detailing v0.8.0 and v0.9.0 releases.

Fixes: #208
Fixes: #211

Makes sense!

Could you please update dist/cleanup.js as well by running npm run build?

Makes sense!

Could you please update dist/cleanup.js as well by running npm run build?

Thanks for the gentle reminder @mpdude.

Updated via docker using:

$ docker run \
  --interactive \
  --rm \
  --tty \
  --volume ${PWD}:/var/task \
  --workdir /var/task \
  node:16-buster \
  sh -c 'yarn install && npm run build'

Please advise if the update looks incorrect or if you'd like me to use a different version of node or run a modified command.

@mpdude Is there anything else blocking this?

I'd love to see this PR released. I've got self-hosted runners with tons of ssh-agent processes hanging around.

This fixes it nicely.

Gentle bump on this PR being merged. This would really help with the cleanup of the ssh-agent processes running in the context of non-containerised environments!

@mpdude, I've rebased my branch from upstream and re-run/updated the dist/ folder.

$ docker run \
    --interactive \
    --rm \
    --tty \
    --volume ${PWD}:/var/task \
    --workdir /var/task \
    node:20-buster \
    sh -c 'npm install -g npm@10.5.0 && yarn install && NODE_OPTIONS=--openssl-legacy-provider npm run build'

Like others above, I'd love to get this into a release.

Also would love to see this one in for our self hosted runners.

Hi guys, I would love to see this one merged also 🙏 🙏

pls merge this pr @mpdude

Have this warning in Post Loads private SSH key as well on Github hosted runners. If this PR fixes it, would love to see it merged

Stopping SSH agent
The "file" argument must be of type string. Received undefined
Error stopping the SSH agent, proceeding anyway

y u no merge dis? 😄

🙏 please merge this PR

@mpdude It'd be nice to see this merged. Anything the community can do to help?

Bring `post` actions step into consistency with `main` for changes introduced in webfactory#154 Without this change, `sshAgentCmd` is undefined when passed to `execFileSync()` during `cleanup` and `post` is never successful.

Actual command: $ docker run \ --interactive \ --rm \ --tty \ --volume ${PWD}:/var/task \ --workdir /var/task \ node:20-buster \ sh -c 'npm install -g npm@10.5.0 && yarn install && NODE_OPTIONS=--openssl-legacy-provider npm run build'

does anyone have a stable fork with this fix?

@mpdude Hi, I hope you're doing well and I really hope this doesn't bother you, but are there any plans to merge this fix?

Hi everyone! What's missing for this PR to be merged? Is there anything to help?
Because of this bug, a lot of ssh-agent processes are piling up on my stand-alone GHA runner. I'd be happy to help if there is anything I can do.

Refactored version of #183. Fixes: #208

Thanks for the contribution :) We looked through your PR and just merged a refactored version with #235.

* Update CHANGELOG.md * Acknowledge custom command inputs in cleanup.js (webfactory#235) Refactored version of webfactory#183. Fixes: webfactory#208 * Improve documentation on why we use os.userInfo() (use correct syscall name) Co-authored-by: Matthias Pigulla <mp@webfactory.de> * Release v0.9.1 * use node24 (webfactory#243) Node v20 is going to be EOL in April 2026 -- https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ Resolves webfactory#242 --------- Co-authored-by: Jano Paetzold <jep@webfactory.de> Co-authored-by: Matthias Pigulla <mp@webfactory.de> Co-authored-by: James McGonegal <11491046+jimmymcpeter@users.noreply.github.com>

mpdude approved these changes May 23, 2023

View reviewed changes

cardoe mentioned this pull request Jan 17, 2024

support GitHub Enterprise servers #163

Open

thazhemadam mentioned this pull request Feb 19, 2024

Post cleanup fails #208

Closed

tkalus force-pushed the fix/cleanup_consistency branch from 19c9e1b to 6c290a5 Compare March 4, 2024 19:02

mark-pictor-csec mentioned this pull request Sep 19, 2024

fix: incorrect sshagent command variable name #203

Closed

tkalus added 3 commits September 24, 2024 16:29

chore: Update CHANGELOG

5e1c7c1

fix: Add Cmd Default/Input support to cleanup

83c3be0

Bring `post` actions step into consistency with `main` for changes introduced in webfactory#154 Without this change, `sshAgentCmd` is undefined when passed to `execFileSync()` during `cleanup` and `post` is never successful.

chore: update dist files with npm run build

a767bb0

Actual command: $ docker run \ --interactive \ --rm \ --tty \ --volume ${PWD}:/var/task \ --workdir /var/task \ node:20-buster \ sh -c 'npm install -g npm@10.5.0 && yarn install && NODE_OPTIONS=--openssl-legacy-provider npm run build'

tkalus force-pushed the fix/cleanup_consistency branch from 6c290a5 to a767bb0 Compare September 24, 2024 23:32

tkalus requested a review from mpdude September 29, 2024 00:53

janopae mentioned this pull request Jan 8, 2025

Acknowledge custom command inputs in cleanup.js #235

Merged

janopae added a commit that referenced this pull request Jan 8, 2025

Acknowledge custom command inputs in cleanup.js (#235 )

e3f1a8e

Refactored version of #183. Fixes: #208

janopae closed this Jan 8, 2025

fix: Add Cmd Default/Input support to cleanup by tkalus · Pull Request #183 · webfactory/ssh-agent · GitHub

Uh oh!

fix: Add Cmd Default/Input support to cleanup - #183

fix: Add Cmd Default/Input support to cleanup#183tkalus wants to merge 3 commits intowebfactory:masterwebfactory/ssh-agent:masterfrom tkalus-forks:fix/cleanup_consistencytkalus-forks/ssh-agent:fix/cleanup_consistencyCopy head branch name to clipboard

Conversation

tkalus commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpdude left a comment

Choose a reason for hiding this comment

Uh oh!

tkalus commented May 23, 2023

Uh oh!

tkalus commented Jun 1, 2023

Uh oh!

sdt commented Aug 16, 2023

Uh oh!

thazhemadam commented Feb 15, 2024

Uh oh!

tkalus commented Mar 4, 2024

Uh oh!

sambanks commented Mar 18, 2024

Uh oh!

edelanghe-ledger commented May 15, 2024

Uh oh!

bvnp43 commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeromecoupe commented Jun 14, 2024

Uh oh!

bobanj commented Jul 10, 2024

Uh oh!

bpedman commented Aug 2, 2024

Uh oh!

mark-pictor-csec commented Sep 19, 2024

Uh oh!

sanderpick commented Oct 22, 2024

Uh oh!

hcrespozemoga commented Dec 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serdardalgic commented Dec 31, 2024

Uh oh!

janopae commented Jan 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

fix: Add Cmd Default/Input support to cleanup#183
tkalus wants to merge 3 commits into
webfactory:masterfrom
tkalus-forks:fix/cleanup_consistency

tkalus commented May 23, 2023 •
edited
Loading

bvnp43 commented May 21, 2024 •
edited
Loading

hcrespozemoga commented Dec 27, 2024 •
edited
Loading