Skip to content

fix bug #1183 prompt validation#37

Merged
isolomatov-gd merged 1 commit intomainfrom
validate-prompts-fix
Mar 25, 2026
Merged

fix bug #1183 prompt validation#37
isolomatov-gd merged 1 commit intomainfrom
validate-prompts-fix

Conversation

@k-bieniek
Copy link
Contributor

@k-bieniek k-bieniek commented Mar 22, 2026

Summary

Fixes prompt validation workflow failures when triggered outside of a pull request context (e.g., workflow_dispatch or push events).

Problem

The validate-prompts workflow assumed github.event.pull_request.base.ref was always set, causing failures when:

  • The workflow runs via workflow_dispatch (manual trigger) where base.ref is empty
  • git diff exits non-zero when no matching instruction files are found, breaking subsequent steps
  • Generate PR comment and Post PR comment steps attempted to post to a PR that does not exist in non-PR runs

Changes

  • Fallback base ref: When base.ref is not available (non-PR context), falls back to HEAD^ instead of crashing on an empty BASE_BRANCH
  • Error-safe diff: git diff now uses 2>/dev/null || true to prevent step failure when no .md files match
  • PR-only comment steps: Generate PR comment and Post PR comment are now gated with github.event_name == 'pull_request' so they are skipped gracefully on non-PR triggers
  • Consistent ref variable: BASE_BRANCH env var renamed to BASE_REF; the resolved ref is propagated via step output (base_ref) so later steps handle both branch-name refs (origin/<branch>) and the HEAD^ fallback correctly

Testing

  • PR trigger: existing behavior preserved — git fetch + diff against origin/<base> as before
  • Manual dispatch / push: workflow now completes without error; comment steps are skipped

Fixes #1183

@isolomatov-gd
Copy link
Collaborator

/rosetta review this PR please

@github-actions
Copy link
Contributor

Rosetta Triage Review

Summary: This PR fixes a bug in the validate-prompts.yml workflow where github.event.pull_request.base.ref was used unconditionally, causing failures when the workflow runs outside a pull_request event context (e.g., push or manual dispatch). It also adds a guard on the PR comment steps so they only run during pull_request events.

Findings:

  • Fix is correct: The conditional check for github.event.pull_request.base.ref with a HEAD^ fallback properly handles non-PR triggers. The || true on the git diff command prevents pipeline failures when no matching files are found.
  • Output wiring: The new base_ref output is correctly threaded from changed-files step to the validation and comment steps — replaces the previously unused BASE_BRANCH env variable.
  • PR comment guard: Adding github.event_name == 'pull_request' to the Generate PR comment and Post PR comment conditions is the right fix — these steps require a PR context to post comments.
  • Empty PR body: The PR description is blank. For traceability, it should reference what bug #1183 is and briefly describe the root cause and fix.

Suggestions:

  • Add a one-paragraph PR description explaining the root cause (workflow triggered outside PR context), the symptom (job failure), and the fix (conditional base ref + event guard).
  • Consider whether HEAD^ is the right fallback for all non-PR triggers — on a fresh branch with a single commit or a squash-merge scenario it may produce unexpected diffs. origin/main or origin/${{ github.event.repository.default_branch }} could be a safer fallback if the workflow is triggered by push to non-main branches.
  • The 2>/dev/null suppresses errors on the git show call; this is fine since the outer if block already handles the missing-base-version case gracefully.

Automated triage by Rosetta agent

@isolomatov-gd
Copy link
Collaborator

/rosetta please update PR description

@github-actions
Copy link
Contributor

Rosetta — PR Description Updated

I've updated the PR description based on the diff analysis. Here's what was added:

Summary: Fixes prompt validation failures when triggered outside a PR context (manual dispatch, push events).

Problem documented:

  • github.event.pull_request.base.ref is empty on non-PR triggers, causing git fetch to fail
  • git diff could exit non-zero with no matching files, breaking the step
  • Comment-posting steps attempted to post to a non-existent PR on non-PR runs

Changes documented:

  • Fallback to HEAD^ when base.ref is not set
  • git diff with 2>/dev/null || true for graceful no-match handling
  • Generate PR comment and Post PR comment gated on github.event_name == 'pull_request'
  • BASE_BRANCHBASE_REF with step-output propagation for consistent ref handling

Automated triage by Rosetta agent

@k-bieniek k-bieniek marked this pull request as ready for review March 24, 2026 11:39
@github-actions github-actions bot added the bug Something isn't working label Mar 24, 2026
@github-actions
Copy link
Contributor

Rosetta Triage Review

Summary: This PR fixes the validate-prompts GitHub Actions workflow to handle non-PR trigger contexts (e.g., workflow_dispatch, push events) by adding a fallback base ref, suppressing non-zero git diff exit codes, and gating PR comment steps behind an event_name == 'pull_request' condition.

Findings:

  • Fix is well-scoped and addresses the root causes clearly: missing base ref, unsafe git diff exit, and unconditional PR comment steps
  • Renaming BASE_BRANCH to BASE_REF is semantically accurate since it can now hold either a branch name or HEAD^
  • Step output propagation (echo "base_ref=$BASE_REF" >> $GITHUB_OUTPUT) is correct and consistent
  • 2>/dev/null || true is the appropriate pattern for suppressing git diff failures when no matching files are found
  • PR description clearly explains the problem, changes, and manual test scenarios

Suggestions (optional):

  • Minor edge case: the HEAD^ fallback will fail on a repository with only a single commit (no parent). Consider using git rev-parse --verify HEAD^ 2>/dev/null && echo HEAD^ || echo 4b825dc642cb6eb9a060e54bf8d69288fbee4904 or git diff --name-only $BASE_REF 2>/dev/null || true (single-dot) as a further safeguard, though this is unlikely to affect this repo in practice

Automated triage by Rosetta agent

@isolomatov-gd isolomatov-gd merged commit 74b5e98 into main Mar 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants