Skip to content

fix(openemr-cmd): avoid SIGPIPE in worktree validation under pipefail#784

Merged
kojiromike merged 3 commits into
openemr:masterfrom
kojiromike:openemr-cmd-pipefail-sigpipe
Jun 6, 2026
Merged

fix(openemr-cmd): avoid SIGPIPE in worktree validation under pipefail#784
kojiromike merged 3 commits into
openemr:masterfrom
kojiromike:openemr-cmd-pipefail-sigpipe

Conversation

@kojiromike
Copy link
Copy Markdown
Member

Summary

openemr-cmd worktree list and worktree remove can fail on setups with many git worktrees: list shows (no worktrees) even when entries exist, and remove aborts with Path '...' is not a registered git worktree.

Root cause

wt_validate_dir and wt_validate_dir_safe validate a path by piping git's worktree listing into grep:

git -C "${OPENEMR_ROOT}" worktree list --porcelain 2>/dev/null \
    | grep -Fqx "worktree ${dir_real}"

The script runs under set -o pipefail. grep -q exits as soon as it finds a match and closes the read end of the pipe. If git is still writing (its output exceeds the ~64KB pipe buffer, which happens once you have many worktrees), git receives SIGPIPE and exits 141. Under pipefail the pipeline's exit status becomes that 141, so the if ! branch fires and a path that is a registered worktree gets rejected.

This stays latent on typical setups where git's output fits in the pipe buffer and git finishes writing before grep exits — the SIGPIPE never happens.

Fix

Capture the listing into a variable first, then match with a herestring. No pipe means no SIGPIPE:

local worktrees
worktrees=$(git -C "${OPENEMR_ROOT}" worktree list --porcelain 2>/dev/null)
if ! grep -Fqx "worktree ${dir_real}" <<< "${worktrees}"; then
    ...
fi

Test plan

  • shellcheck clean under the repo's .shellcheckrc (enable=all)
  • bash -n passes
  • Reproduced the failure on a machine with ~150 worktrees: the old pipe form fails under pipefail (pipeline exits 141); the herestring form returns 0
  • End-to-end on macOS: worktree list shows the entry and worktree remove completes cleanly (both previously broken)

Note

Found while verifying #783 (the macOS realpath fix). That fix let execution finally reach this validation code, surfacing this separate, platform-independent bug. The two are independent; this PR stands alone.

Copilot AI review requested due to automatic review settings June 5, 2026 19:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a correctness issue in openemr-cmd worktree validation when running under set -o pipefail with many worktrees, by eliminating a git ... | grep -q pipeline that can trigger SIGPIPE and produce false negatives.

Changes:

  • Replace git worktree list --porcelain | grep -Fqx ... with a “capture then grep via herestring” approach in wt_validate_dir and wt_validate_dir_safe.
  • Add explanatory comments documenting the SIGPIPE/pipefail failure mode.
  • Bump script version to 1.0.40.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread utilities/openemr-cmd/openemr-cmd Outdated
Comment thread utilities/openemr-cmd/openemr-cmd Outdated
wt_validate_dir and wt_validate_dir_safe piped 'git worktree list
--porcelain' into 'grep -Fqx'. Under 'set -o pipefail', grep -q exits on
the first match and closes the pipe; git, still writing the remaining
entries, takes SIGPIPE and exits 141. pipefail then reports the whole
pipeline as failed, so a path that IS a registered worktree is rejected.

This only triggers once git's output exceeds the pipe buffer (~64KB),
i.e. with many worktrees, which is why it stays latent on typical setups.
The symptom is 'worktree list' showing "(no worktrees)" and 'worktree
remove' aborting with "is not a registered git worktree".

Capture the listing into a variable and match it with a herestring,
eliminating the pipe (and the SIGPIPE) entirely.

Assisted-by: Claude Code
… fix

The pipefail/SIGPIPE fix added 7 net lines above the function-definition
boundary, so sourcing head -n 1446 cut mid-function and every pure-helper
test failed with "syntax error: unexpected end of file". Move the boundary
to 1452, the closing brace of the last function before USAGE_EXIT_CODE.

Assisted-by: Claude Code
@kojiromike kojiromike force-pushed the openemr-cmd-pipefail-sigpipe branch from 06c7694 to 538e505 Compare June 6, 2026 01:19
Splitting the 'git worktree list | grep' pipeline into a standalone
command substitution changed its set -e behavior: previously git's exit
status was masked inside the 'if ! ...' test, but a bare assignment now
aborts the script if git fails. Restore the original contract — wt_die
with a clear message in the fatal variant, 'return 1' in the safe one —
by handling the substitution's exit explicitly. Bump OC_SCRIPT_FUNCS_END
for the added continuation line.

Assisted-by: Claude Code
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@kojiromike kojiromike merged commit 3eddc62 into openemr:master Jun 6, 2026
7 checks passed
@kojiromike kojiromike deleted the openemr-cmd-pipefail-sigpipe branch June 6, 2026 02:19
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