Skip to content

fix: check for git directly instead of relying on CI=true for lefthook install - #282

Open
panish16 wants to merge 2 commits into
mainfrom
fix/lefthook-ci-skip
Open

panish16 wants to merge 2 commits into
mainfrom
fix/lefthook-ci-skip

Conversation

@panish16

Copy link
Copy Markdown
Collaborator

Improves on #281's guard, per feedback from Thor and Patrick that lefthook shouldn't be attempting to run in CI/CD at all, not just fail-safe.

What changed

- "prepare": "lefthook install || exit 0"
+ "prepare": "command -v git >/dev/null 2>&1 && lefthook install || true"

Why not CI=true, which was the first thing tried

lefthook's own npm postinstall already skips itself under CI=true by default. Tried making prepare do the same ([ "$CI" = "true" ] || lefthook install) and pushed it through a real CD run first rather than assuming - it still hit the original crash. Google Cloud Build does not set CI=true in the build step, so that check was always false there. GitHub Actions does set it automatically, but this app's actual build runs inside a Cloud Build step invoked from within the Action, and that inner environment doesn't inherit the Action's env.

Checking for git directly avoids depending on any particular CI system's conventions and matches the actual failure mode.

Verified

Pushed this exact change through a real CD run (dev target): install completes with prepare: Done and no git: executable file not found line at all now, then deployed successfully to https://yfthig-dev.web.app.

lefthook's own npm postinstall already skips itself under CI=true by
default (only installs if LEFTHOOK=1/true is explicitly set). Our
prepare script called 'lefthook install' directly, which bypasses
that and always runs regardless of environment.

'|| exit 0' stopped the crash but still attempted the install in CI
every time. This skips it outright, since hooks have no purpose in a
non-interactive build. Same fix already applied on bcgov/namex#2019.
Google Cloud Build does not set CI=true (confirmed via a real CD run
with the previous version of this fix - it still hit the git-not-found
crash, since the [ "$CI" = "true" ] check was always false there).
GitHub Actions sets CI=true automatically but this app's actual build
happens inside a Cloud Build step invoked from within the Action, and
that inner environment doesn't inherit it.

Check for git's actual presence instead, which is what we care about
and doesn't depend on any particular CI system's conventions.
panish16 added a commit to panish16/namex that referenced this pull request Sep 25, 2026
CI=true isn't a safe assumption across CI systems - confirmed on
bcgov/bcregistry#282 that Google Cloud Build doesn't set it at all,
so that version of this guard would have silently kept failing there
despite looking correct. Checking for git's actual presence instead
doesn't depend on any CI provider's conventions.
@panish16

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@bcregistry-sre

Copy link
Copy Markdown
Collaborator

Temporary Url for review: https://yfthig-dev--pr-282-g8xemlex.web.app

This branch was successfully deployed

1 active deployment
dev — f10dcf18 Deployed Sep 25, 2026 by panish16 via bcregistry-ui-cd / deploy #288
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.

2 participants