Skip to content

emrg: scripts/push-branch-from-api.py — push local branch via Git Data API when git-over-https is down - #989

Merged
argszero merged 4 commits into
masterfrom
feature/push-branch-from-api
Aug 25, 2026
Merged

emrg: scripts/push-branch-from-api.py — push local branch via Git Data API when git-over-https is down#989
argszero merged 4 commits into
masterfrom
feature/push-branch-from-api

Conversation

@pm25coder

Copy link
Copy Markdown
Collaborator

Summary

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260826-031357 (review 1/3)

Reviewed the full diff (3 files, +814/−1): scripts/push-branch-from-api.py is the push-direction counterpart of #988's sync-master-from-api.py. Verified:

  • Hermetic test suite (8 tests, faithful fake API recomputing every sha with real git hash-object/mktree) — ran locally in a clean worktree at PR head aeae993, 8/8 passed.
  • All four Git Data endpoints wired (blobs/trees/commits/refs), bottom-up object upload, commit parents chained through remote shas (multi-commit fix), ref update last with fail-loud isolation ("no refs touched" on any pre-ref failure).
  • Gotchas from the 2026-08-26 00:59 outage encoded: byte-exact blob upload, ls-tree -r nesting, +0800 offset preservation (parse_commit test asserts 2023-11-15T06:13:20+08:00), UTF-8/errors=replace text I/O.
  • Remote sha materialization via _raw_commit + git hash-object -w, local ref rewrite, and GET-ref + git diff content-identical verification — matches the #988 reconstruction rigor.
  • CI double-green (run 32885775077: test + test-windows); Agent.md doc count 1087 → 1095 consistent (8 new tests).

@pm25coder

Copy link
Copy Markdown
CollaboratorAuthor

Added one follow-up commit (413e9a3) after production dogfooding: the push script now materializes every created commit locally (not just the head) via the reconstructed raw objects + git hash-object -w, so git log traverses the new branch head through the API-normalized intermediate commits instead of failing on missing parents. Covered by a new assertion in the e2e test (git log --oneline refs/heads/<branch> must list all commits after the push). Full suite still 1030 passed / 65 skipped / 1095 collected; CI re-running on the new head.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260826-035632 (review 2/3, independent re-review of new head)

Head moved aeae993413e9a3 (follow-up after production dogfooding). Reviewed the delta (+23/−13):

  • Fix: materialize EVERY created remote commit locally (payloads list zipped with chain), not just the last — previously a multi-commit push left intermediate API-normalized commits absent from the local object store, so git log could not traverse the new branch head. Each materialized sha is verified against commit_map before update-ref; fail-loud on mismatch with the fetch fallback hint.
  • Test: end-to-end now asserts git log --oneline on the rewritten branch traverses both commits (parents complete locally).
  • Re-verified at new head: 8/8 hermetic tests pass in a clean worktree; Agent.md doc count 1095 unchanged (assertion-only change); CI double-green on 413e9a3 (run 32889852360: test + test-windows); MERGEABLE/CLEAN.

Prior review (cyc20260826-031357, 1/3) remains valid; 1/3 + 2/3 consecutive, no ❌.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260826-040314 (review 3/3)

Head unchanged since 2/3 (413e9a3); CI still double-green (run 32889852360); MERGEABLE/CLEAN. Prior reviews from cycles cyc20260826-031357 (full 8/8 hermetic pass) + cyc20260826-035632 (delta +23/−13 re-verification) remain valid. Three consecutive LGTMs from distinct cycles, no ❌ in between — mergeable.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@pm25coder@argszero