fix: stricter regex and better error logging for sync PR commit type - #70
Conversation
- Require colon (with optional scope) after type keyword to avoid false positives like "feature: ..." or "testing out ..." - Log warning instead of silently swallowing gh api failures Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughWorkflow step updated to capture Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR tightens the conventional-commit extraction in the Changes
Finding
Confidence Score: 4/5Safe to merge — functional behavior (commit-type extraction and fix fallback) is correct; the only finding is that the warning may be silently skipped for error types beyond Not Found and Bad credentials. The regex fix and fallback logic are correct and address the original false-positive and silent-failure issues from #69. The remaining P2 finding is about incomplete error-message pattern matching for the warning path — the fallback to fix is unaffected — so it does not block correctness but is worth addressing per the PR's own stated goal. .github/workflows/on-child-update.yml — error-detection block (lines 41–44)
|
| Filename | Overview |
|---|---|
| .github/workflows/on-child-update.yml | Tightened commit-type regex to require type(scope)?: format and replaced silent 2>/dev/null with 2>&1 plus an error-check block; the error check covers "Not Found" and "Bad credentials" but misses other gh api failure modes (rate limits, missing scopes, network errors), meaning the warning can be silently skipped even though the fix fallback still works correctly. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["gh api fetch commit message, 2>&1 | head -1"] --> B{"COMMIT_MSG empty,\ncontains Not Found,\nor Bad credentials?"}
B -- Yes --> C["Log warning, set COMMIT_MSG to empty"]
B -- No --> D["Use COMMIT_MSG as-is"]
C --> E["Regex: type(scope)?:"]
D --> E
E --> F{Match found?}
F -- Yes --> G["Extract type: feat, fix, perf, refactor, ..."]
F -- No --> H["Default: fix"]
G --> I{"Type is feat, fix, or perf?"}
H --> J["Build PR title: type(repo): sync short-sha"]
I -- Yes --> J
I -- No --> K["Promote to fix"]
K --> J
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/on-child-update.yml
Line: 41-44
Comment:
**Incomplete API error detection**
The warning check only matches `"Not Found"` and `"Bad credentials"`. Several common `gh api` failure modes produce different messages and will silently bypass the warning:
- **Rate limiting**: `"API rate limit exceeded for ..."`
- **Missing auth scopes**: `"Resource not accessible by integration"`
- **Network-level failures**: `gh` may print something like `error connecting to github.com/ghapi`
- **Bad `--jq` on an error response**: `gh` may output `null` to stdout if `.commit.message` is absent in the error body
In all these cases, `COMMIT_MSG` will contain neither `"Not Found"` nor `"Bad credentials"`, so the warning is skipped. The fallback to `fix` still works (the regex simply won't match), but the warning — which is the stated goal of this fix — is silently suppressed.
A more robust approach is to capture the exit code separately and use it as the primary signal:
```bash
COMMIT_MSG=$(gh api "repos/harmony-labs/${REPO_NAME}/commits/${FULL_SHA}" \
--jq '.commit.message' 2>/tmp/gh_err | head -1)
GH_EXIT=$?
if [[ $GH_EXIT -ne 0 || -z "$COMMIT_MSG" ]]; then
echo "Warning: could not fetch commit message for ${REPO_NAME}@${FULL_SHA} (exit $GH_EXIT: $(cat /tmp/gh_err | head -1)), defaulting to fix"
COMMIT_MSG=""
fi
```
This way any `gh api` failure — regardless of the specific error message — triggers the warning.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: stricter regex and better error log..." | Re-trigger Greptile
Check gh api exit code instead of pattern-matching specific error strings. Captures stderr to a temp file for the warning message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/on-child-update.yml:
- Around line 40-44: The pipeline's exit code is being lost by piping to head;
change the logic so the gh api exit status is captured before head runs: run gh
api and store its raw output into a temp variable (e.g., RAW_MSG=$(gh api
"repos/harmony-labs/${REPO_NAME}/commits/${FULL_SHA}" --jq '.commit.message'
2>"$GH_ERR"); GH_EXIT=$?), then derive COMMIT_MSG from that raw variable
(COMMIT_MSG=$(printf '%s' "$RAW_MSG" | head -1)); alternatively enable set -o
pipefail near the top so PIPESTATUS is respected; update references to
COMMIT_MSG, GH_EXIT, gh api and GH_ERR accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6cf68b5b-374c-4d16-a3b8-b5ae8befc675
📒 Files selected for processing (1)
.github/workflows/on-child-update.yml
The pipe to head masked gh api's exit code ($? was always 0). Split into two steps: capture raw output + exit code first, then extract first line separately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Addresses review comments from #69:
:after the type (e.g."feature: add login"matched asfeat). Now requirestype(scope)?:format.2>/dev/nullswallowed errors fromgh api. Now logs a warning when commit message fetch fails.Test plan
"fix: some change"→ extractsfix"feat(scope): thing"→ extractsfeat"feature: wrong keyword"→ no match → defaults tofix"testing the pipeline"→ no match → defaults tofixfix🤖 Generated with Claude Code
Summary by CodeRabbit