Report the pushed commit SHA from report_progress - #25
Merged
Conversation
report_progress commits and pushes, but the resulting commit SHA was never captured, so consumers had no way to persist a "last pushed commit SHA". - git.ts: add commitSha to CommitAndPushResult, resolved via `git rev-parse HEAD` after the push succeeds. The lookup is isolated in its own try/catch so a rev-parse failure can never turn an already-successful push into a reported error, which a caller could retry into a duplicate commit. - client.ts: accept commitSha on sendReportProgress and emit it as commit_sha, using the existing `!== undefined` guard so the key stays absent when unknown. - mcp-server.ts: thread the SHA from commitAndPush into sendReportProgress. It stays undefined on the local-only and error paths, so a SHA that was not actually pushed is never sent. Closes #24 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds reporting of the successfully pushed commit SHA to downstream platform consumers.
Changes:
- Resolves
HEADafter a successful push without failing the operation if lookup fails. - Adds
commit_shato progress payloads when available. - Threads the SHA through the MCP progress-reporting flow.
Show a summary per file
| File | Description |
|---|---|
src/git.ts |
Captures the pushed commit SHA. |
src/client.ts |
Adds the SHA to progress payloads. |
src/mcp-server.ts |
Forwards the pushed SHA to the platform client. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
ritchxu
approved these changes
Aug 10, 2026
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
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 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.
Closes #24.
What
report_progresscommits and pushes, but the resulting commit SHA was never captured, so downstream consumers had no way to persist a "last pushed commit SHA" — the value was structurally unobtainable. github/sweagentd#15129 adds the server-side plumbing and currently sees an emptycommit_sha100% of the time.Three additive changes:
src/git.ts— addcommitSha?: stringtoCommitAndPushResult, resolved viagit rev-parse HEADafterpushWithRebaseFallbacksucceeds, so the SHA always refers to a commit that is actually on the remote.src/client.ts— widensendReportProgressto acceptcommitShaand emit it ascommit_sha, using the existing!== undefinedguard style.src/mcp-server.ts— threadgitResult.commitShainto thesendReportProgresscall.Safety notes
The
rev-parseis deliberately isolated in its own try/catch. If it shared the caller's try, arev-parsefailure would report an already-successful push as an error, which can trigger a retry and a duplicate commit. On failure we log and leavecommitShaundefined.In
mcp-server.tsthe SHA staysundefinedon both the local-only (config.push === false) and error paths, so a SHA that was not actually pushed is never sent.Backward compatibility
Additive and safe in either rollout order. Absent keys are omitted from
contentObj, so a pre-fix SDK sends content with nocommit_shaand the server decodes it to"", skipping the update behind its existing!= ""guard. Servers that don't understandcommit_shaare unaffected.Verification
npm run typecheckandnpm run buildboth pass. The repo has no test framework (notestscript, no test files, no test runner indevDependencies), so rather than scaffolding one I verified behavior against real git repositories:commitAndPushreturns the SHA, and that SHA is confirmed present on the remote after the push.gitwhere onlyrev-parse HEADfails: the push still succeeds,commitShais absent, and the branch has exactly 1 commit — no duplicate.sendReportProgressomitscommit_shaentirely when it is unset or explicitlyundefined, and includes it when known.Open questions for maintainers
package.jsonis at0.1.0whilegithub/copilot-agent-runtimeandsweagentd/agent-runtimeboth pin"@github/copilot-engine-sdk": "0.2.0"— worth confirming the intended next published version when cutting the release that carries this fix.finalizeChanges. That post-agent-loop safety net callscommitAndPushwith noreport_progressevent afterwards, so the last reported SHA can still lag the true final SHA whenever that path fires. Left as-is here; worth deciding whether that push should report its SHA too.