fix: suppress spurious error annotations in push_repo_memory for expected git failures#19979
fix: suppress spurious error annotations in push_repo_memory for expected git failures#19979
Conversation
…po-memory Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces noisy GitHub Actions error annotations produced by push_repo_memory during first-run scenarios where certain git failures are expected (e.g., the memory branch doesn’t exist yet).
Changes:
- Added a
suppressLogsflag toexecGitSyncto route expected-failure details tocore.debuginstead ofcore.error. - Updated
push_repo_memory.cjsto usesuppressLogs: truefor git operations where failure is expected, and downgraded the pull-failure message from warning to info. - Added unit tests ensuring
suppressLogspreventscore.errorcalls and usescore.debugfor expected failures.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/git_helpers.cjs | Introduces suppressLogs option and changes failure logging behavior to avoid error annotations for expected git failures. |
| actions/setup/js/push_repo_memory.cjs | Applies suppressLogs: true to expected-failure git calls and downgrades pull-failure logging to core.info. |
| actions/setup/js/git_helpers.test.cjs | Adds tests covering the new suppressLogs behavior (debug vs error logging). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot core is always available because a shim.cjs is loaded if needed. Update JavaScript. |
…ilures Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 92641df. Removed all |
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Commit pushed:
|
|
🤖 Smoke test §22807560248 —
Author:
|
There was a problem hiding this comment.
This PR cleanly fixes the spurious error annotations on first-run scenarios. The suppressLogs option is well-designed — destructured before spreading avoids polluting spawnSync, and spawn-level errors still always surface via core.error. The test additions in git_helpers.test.cjs give good coverage of both the suppressed and unsuppressed paths.
📰 BREAKING: Report filed by Smoke Copilot
| // spurious GitHub Actions error annotations for expected failures (e.g., when | ||
| // a branch does not yet exist). | ||
| const { suppressLogs = false, ...spawnOptions } = options; | ||
|
|
There was a problem hiding this comment.
Nice pattern — destructuring suppressLogs before spreading ...spawnOptions ensures the custom flag never leaks into spawnSync options. Clean and safe.
| } catch (error) { | ||
| // Pull might fail if branch doesn't exist yet or on conflicts - this is acceptable | ||
| core.warning(`Pull failed (this may be expected): ${getErrorMessage(error)}`); | ||
| core.info(`Pull failed (this is expected when branch does not exist yet): ${getErrorMessage(error)}`); |
There was a problem hiding this comment.
Downgrading from core.warning to core.info here is correct — a pull failing on first run (no branch yet) is expected behavior, not a warning. This prevents unnecessary orange annotations in the Actions UI.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test Results - Run §22807560229Overall: PARTIAL (1 failed, 2 skipped) Core Tests (#1-10): ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ PR Review Tests (#11-17): ✅ ✅ ✅
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
On first run,
push_repo_memorygenerates 5 GitHub Actions error annotations fromgit fetchandgit pullfailures that are entirely expected — the memory branch simply doesn't exist yet. These show up as real errors in the annotations UI, causing confusion.Root cause
execGitSyncunconditionally callscore.error(...)on any non-zero git exit, even when the caller catches and handles the failure. That produces GitHub Actions annotations regardless of whether the failure was anticipated.Changes
git_helpers.cjs— AddedsuppressLogsoption toexecGitSync. Whentrue, non-zero exit-status failure details are routed tocore.debuginstead ofcore.error, suppressing annotations while preserving debuggability. Spawn-level errors (result.error, e.g. ENOENT/EACCES) always usecore.errorregardless ofsuppressLogssince those are unexpected system failures. Removed unnecessarytypeof core !== "undefined"guards —coreis always available viashim.cjs. The option is destructured out before spreading intospawnSyncto avoid passing an unknown option to it.push_repo_memory.cjs— AppliedsuppressLogs: trueto three call sites where failure is expected:sparse-checkout disable(no-op when sparse checkout isn't active)git fetchfor branch existence probe (fails on first run)git pullbefore push (fails on first run when branch doesn't exist)core.warningtocore.info— it's not a warning, it's normal.git_helpers.test.cjs— Added tests verifyingsuppressLogs: truepreventscore.errorcalls (for non-zero exits) and routes details tocore.debuginstead. Added abeforeEach/afterEachthat provides a minimalcorestub so all tests pass without requiring manualglobal.coresetup.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Changeset
git fetch/git pullfailures duringpush_repo_memoryso they don’t produce GitHub Actions error annotations.Warning
The following domains were blocked by the firewall during workflow execution:
ab.chatgpt.comgithub.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
✨ PR Review Safe Output Test - Run 22807560229