Uh oh!
There was an error while loading. Please reload this page.
fix(pr-followup): inject a server-fetched CI log excerpt into pr-fix feedback - #601
Merged
Conversation
…d stub Failing-check feedback was event.body || 'Check "X" concluded failure'. But these CI jobs (e.g. windowstead's Godot suite) publish no check-run output.summary, so event.body is empty and the coder received a contentless 'check failed' with zero error detail — it burned every pr-fix attempt guessing blind (esp. macOS jobs it can't run locally). Same models fixed these fine under openclaw, which fed them the actual logs. Build the feedback from the reason + the job-log URL + a copy-pasteable python3 recipe to fetch and grep it (the sandbox has python3 + $GITHUB_TOKEN, no curl/gh). The coder pulls the real error itself instead of being force-fed the whole log. Requires the coder token to carry actions:read.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…feedback Supersedes the earlier pull approach: the coder token is classic public_repo (kept minimal — classic is required for fork PRs to LLMKube), and reading Actions logs needs full `repo`, which is too broad. So fetch the log server-side with dispatch's own credential and hand the coder a bounded error excerpt instead. - github.ts: fetchFailedJobLogExcerpt (manual-redirect to the signed blob so the auth header isn't forwarded) + extractLogExcerpt (pure: strips timestamps, finds the last error marker, returns a bounded window, skips cleanup noise) + jobIdFromCheckRunUrl. - sync: fetch the excerpt per failing check and put it in the event body. - ingestion: feedback = reason + log excerpt + log URL; degrades to reason + URL when no excerpt (e.g. dispatch lacks Actions:read). Requires dispatch's GitHub App to have Actions:read (server-side, scoped — does NOT touch the coder's fork-PR token). Coder token unchanged.
…i-logs # Conflicts: # src/app/api/pr-followup/sync/route.ts # src/lib/github.ts
Contributor
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: public_route_changes)
PR PR 601 Review: inject a server-fetched CI log excerpt into pr-fix feedback
Recommendation: Approve
This PR enhances CI failure feedback by fetching and including actual job log excerpts (rather than empty summaries) when GitHub Actions checks fail. The implementation is clean, well-tested, and follows repository conventions.
Change-by-Change Findings
src/lib/github.ts — New functions for log extraction
jobIdFromCheckRunUrl(): Safely extracts the numeric job ID via regex/\/job\/(\d+)/from check-run URLs. Only captures digits, preventing injection. Returnsnullfor malformed/missing URLs.extractLogExcerpt(): Pure function that processes raw log text — strips ISO timestamps, finds last error marker via pattern matching (##[error],::error::,ERROR:,SCRIPT ERROR,AssertionError,Traceback,FAIL, exit codes, etc.), returns a bounded 6000-char window. Falls back to last lines before cleanup noise when no marker found. No filesystem operations — only string processing.fetchFailedJobLogExcerpt(): Fetches the GitHub Actions job log, manually resolves the 302 redirect to the signed blob URL (auth header intentionally not sent per GitHub's signed-URL requirement), and passes the result throughextractLogExcerpt(). Gracefully returns""on any failure (network, missing permissions, malformed response).
src/app/api/pr-followup/sync/route.ts — Integration point
- Calls
jobIdFromCheckRunUrl()oncheckRun.html_url(safely handlesundefined/malformed URLs → returnsnull) - Only fetches excerpt when
jobIdis truthy; otherwise uses""as fallback - Sets
body: excerpt || checkRun.output?.summary || ""— consistent with existing fallback behavior
src/lib/pr-followup-ingestion.ts — Feedback generation
- Uses the fetched excerpt (now in
event.body) to construct richer feedback with the actual error and log URL - Degrades gracefully to "reason + URL" when no excerpt is available
- Improves on the previous
"Check X concluded failure"stub
Test files — github.test.ts, pr-followup-ingestion.test.ts
- Tests for
jobIdFromCheckRunUrl(): valid URL, invalid URL,undefined - Tests for
extractLogExcerpt(): error region extraction, timestamp stripping, empty input, length cap - Integration tests: excerpt-in-feedback, degrade-to-URL fallback
Required Checks
| Check | Status | Notes |
|---|---|---|
| verify route access controls are in place | ✅ Verified | POST /api/pr-followup/sync already imports and uses authorizeRequest from @/lib/auth. No new routes added; existing auth is unchanged. |
| check for unintended public endpoints | ✅ Verified | No new API routes introduced. Only library functions and internal route logic modified. |
| review for path traversal vulnerabilities | ✅ Not applicable | This PR handles GitHub API URLs and job log text processing — not filesystem paths. jobIdFromCheckRunUrl() strictly extracts numeric IDs via regex. extractLogExcerpt() performs pure string manipulation. |
| test with edge-case paths (null bytes, symlinks) | ✅ Not applicable | No filesystem path operations in this PR. Job IDs are validated as numeric strings. |
Standards Compliance
- Error handling: Uses
try/catchwith silent degradation to""(documented behavior for missing Actions:read permission) - Validation: Input URL is validated before extraction;
jobIdis numeric-only by regex design - No new secrets: No credentials added; uses existing
getHeadersAsync()pattern - Test coverage: 113 tests pass including new unit and integration tests
- CI: Build, Typecheck, Lint, Tests, Docker Build — all success
Evidence Provider Findings
No evidence providers were configured for this review.
Tool Harness Findings
No tool harness findings to report.
Unknowns / Needs Verification
None identified. The implementation is fully visible in the diff, tests cover edge cases, and all CI checks pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Failing-check pr-fix feedback was
event.body || 'Check "X" concluded failure'. These CI jobs (windowstead's Godot suite, macOS export/validation) publish no check-runoutput.summary, so the coder received a contentless "check failed" with zero error detail — it burned every attempt guessing blind (all of windowstead's pr-fix workloads exhausted overnight). The same local models fixed these fine under openclaw, which fed them the actual logs.Why server-side fetch (not coder-pull)
The coder token is classic
public_repo— kept minimal, and classic is required because fine-grained PATs can't open fork PRs against upstreams (defilantech/LLMKube). Reading Actions logs needs full classicrepo, which would open every private repo just to read public CI logs. So dispatch fetches the log with its own credential and hands the coder a bounded excerpt; the coder token stays untouched.Change
github.ts:fetchFailedJobLogExcerpt(resolves the logs 302 manually so the signed-blob GET isn't sent the auth header) +extractLogExcerpt(pure: strips timestamps, finds the last error marker, returns a bounded window, skips cleanup noise) +jobIdFromCheckRunUrl.sync/route.ts: fetch a bounded excerpt per failing check, put it in the event body.pr-followup-ingestion.ts: feedback = reason + log excerpt + log URL; degrades to reason + URL when no excerpt.Requires
Verification
vitest: 113 passed across ingestion + github + sync suites (new: excerpt-in-feedback, degrade-to-URL,extractLogExcerpttimestamp-strip/error-region/cap,jobIdFromCheckRunUrl).typecheck+eslintclean.Notes
sync/route.tsin different spots; merge fix(pr-followup): detect merge conflicts in scheduled sync #600 first, this rebases cleanly.