Skip to content

fix(git-sync): auto-sync heartbeat fetches and rebases before push; refuses on a shared source-mode branch (#3011) - #3016

Merged
vybe merged 1 commit into
devfrom
feature/3011-autosync-rebase
Sep 25, 2026
Merged

vybe merged 1 commit into
devfrom
feature/3011-autosync-rebase

Conversation

@dolho

@dolho dolho commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • The auto-sync heartbeat (_run_auto_sync_once) was add -A → commit → push origin HEAD with no fetch, so the first foreign push to the agent's branch failed every later cycle non-fast-forward, forever. It now fetches origin/<branch>, and when behind, rebases --autostash and pushes with --force-with-lease=refs/heads/<branch>:<fetched sha> (a push racing it is rejected, never overwritten; never bare --force). Not behind → the plain push as before; branch absent on the remote → the push creates it.
  • Conflict (or a timeout-killed rebase) → git rebase --abort: repo left exactly as it was, remote untouched, nothing reset or auto-resolved; records diverged: rebase conflict on <branch>, which the existing three-strike sync_failing path raises.
  • Shared-branch refusal: GIT_SOURCE_MODE=true on the repo's default branch (origin/HEAD, else main/master) refuses before committing — refused: source-mode on <branch>. Fork-to-own agents are exempt (GIT_UPSTREAM_REPO, or the upstream remote on the volume, since that env isn't re-derived on recreate).
  • sync-state.json gains behind_after_fetch and last_successful_push_at (inputs for trinity-enterprise#706).
  • Operator sync_to_github path unchanged.

Changes

  • docker/base-image/agent_server/routers/git.py — _is_shared_source_branch, _rebase_onto_remote, _is_missing_remote_ref; reconcile step in _run_auto_sync_once; two sync-state fields
  • tests/unit/test_3011_autosync_rebase.py — 16 tests on real repos with a foreign clone pushing underneath (8 red on the pre-fix cycle)
  • docs/memory/feature-flows/git-sync-health.md (§1b), docs/memory/architecture/agent-lifecycle.md, tests/registry.json

Test Plan

  • cd tests && pytest unit/test_3011_autosync_rebase.py unit/test_agent_server_auto_sync.py unit/test_1595_git_maintenance.py unit/test_2036_claude_settings_leak.py — 79 passed
  • tests/lint_sys_modules.py clean
  • Base image rebuild + live agent on local dev (see comment): foreign push → next cycle rebases and pushes; conflict → diverged; source-mode on main → refused

Fixes #3011

🤖 Generated with Claude Code

…efuses on a shared source-mode branch (#3011)

The heartbeat was add -A -> commit -> push origin HEAD with no fetch, so
the first foreign push to the agent's branch failed every later cycle
non-fast-forward, forever, with the agent's commits piling up locally.

- fetch origin <branch>; if behind, rebase --autostash onto it and push
  with --force-with-lease pinned to the fetched sha (a racing push is
  rejected, never overwritten); a plain push otherwise; a branch not yet
  on the remote is created by the push
- rebase conflict (or timeout): rebase --abort, repo left as it was,
  record 'diverged: rebase conflict on <branch>' for the existing
  sync_failing path; never resolve, overwrite or reset
- source-mode agent on the default branch (origin/HEAD, else main/master)
  refuses before committing: 'refused: source-mode on <branch>';
  fork-to-own agents (GIT_UPSTREAM_REPO or an upstream remote) exempt
- sync-state.json gains behind_after_fetch and last_successful_push_at

Fixes #3011

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@dolho

dolho commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

/review Report

Branch: feature/3011-autosync-rebase → dev (merge-base cec2a64b)
Files Changed: 5 (+507/−20)
Scope: CLEAN. The issue asked for fetch/rebase before push, a conflict abort, the source-mode refusal, a lease-protected push and two new sync-state fields, and the diff delivers exactly those plus tests and docs. Nothing outside the auto-sync cycle is touched.
Plan Completion (issue AC): 5 done · 1 unverifiable (live-agent run, see below)

AC Status Evidence
fetch + rebase --autostash when behind, then push DONE git.py _run_auto_sync_once fetch block; _rebase_onto_remote
conflict → abort, tree untouched, diverged: rebase conflict on <branch> DONE _rebase_onto_remote (abort on non-zero and on TimeoutExpired)
source-mode on default branch → refused, no push DONE _is_shared_source_branch; checked before git add -A, so nothing gets committed either
lease push after rebase, plain push otherwise DONE --force-with-lease=refs/heads/<branch>:<fetched_sha>; never bare --force
last_successful_push_at + behind_after_fetch in sync-state DONE _SYNC_STATE_DEFAULT, _write_sync_state_file
tests DONE tests/unit/test_3011_autosync_rebase.py (16)

Execution coverage

changed symbol executed by live consumer verdict
_run_auto_sync_once reconcile path TestForeignPush, TestConflict, TestLease (real bare remote + foreign clone) auto_sync.py:68 via asyncio.to_thread ✅ executed
_rebase_onto_remote TestConflict (real conflicting commit) _run_auto_sync_once ✅ executed
_is_shared_source_branch TestSourceModeRefusal (6 cases incl. origin/HEAD, env and remote fork-to-own) _run_auto_sync_once ✅ executed
_is_missing_remote_ref test_branch_missing_on_remote_is_created _run_auto_sync_once ✅ executed
sync-state fields asserted in 5 tests GET /api/git/status → sync_state → sync_health_service ✅ executed
test_sync_to_github_does_not_use_the_heartbeat_reconcile inspect.getsource none ⚠️ source-only (see I1)

Fix mutation: with git.py reverted to the merge-base, 8 of the 16 new tests fail: foreign-push rebase, the multi-cycle recovery, behind-only fast-forward, conflict/divergence ×2, lease race, and source-mode refusal ×2. The other 8 are behaviour-preserving cases (not refused, plain push) and are meant to stay green on both sides.

Critical Findings

None.

Informational Findings

[I1] Test gap: the "operator path unchanged" test is a source-text assertion (Confidence: 8)
File: tests/unit/test_3011_autosync_rebase.py (TestOperatorPathUnchanged)
Evidence: src = inspect.getsource(git_router.sync_to_github); assert "_rebase_onto_remote" not in src
Issue: This proves the operator endpoint doesn't name the new helpers. It says nothing about behaviour. The real evidence is that no hunk in this diff touches sync_to_github, and its existing tests still pass.
Suggestion: Drop it, or keep it as a guard and say so in its docstring. It isn't a regression test.

[I2] Behaviour change: an operator-enabled auto-sync on a non-fork source-mode agent now fails every cycle (Confidence: 7)
File: git.py _is_shared_source_branch
Evidence: if _is_shared_source_branch(home_dir, branch): return _fail(f"refused: source-mode on {branch}")
Issue: This is intended per the AC. Still, an owner who ran PUT /git/auto-sync on a source-mode agent built from their own repo (not fork-to-own) used to get pushes to main. Now they get a refused: failure every cycle, a sync_failing queue item after 3 cycles, and, if they opted into freeze_schedules_if_sync_failing (default 0), frozen schedules.
Suggestion: No code change. Call it out in release notes. The operator-queue text already names the reason (refused: source-mode on main).

[I3] Error handling: a failed ahead/behind count after a successful fetch is recorded as behind_after_fetch = 0 (Confidence: 6)
File: git.py _run_auto_sync_once → _compute_ahead_behind
Evidence: _compute_ahead_behind returns (0, 0) on any failure.
Issue: The issue calls this number "the only trustworthy number" (vs #2105), but on a failed count it reads as "not behind". The push then fails non-fast-forward and gets recorded, so nothing is lost; only the metric is wrong for that cycle.
Suggestion: Follow-up with trinity-enterprise#706: record None when the count fails.

Low confidence (appendix)

  • (5) git fetch origin <branch> updates origin/<branch> only through the configured fetch refspec. Every Trinity clone and remote add has the default refspec. A hand-narrowed refspec would make behind read 0 and fall back to the old non-fast-forward failure; that degrades to today's behaviour, not worse. An explicit +refs/heads/<b>:refs/remotes/origin/<b> refspec would remove the dependency.
  • (5) git rebase flattens local merge commits. The agent's own merges are rare, and a conflict during replay aborts cleanly.

Clean Categories

  • SQL/data safety: no DB code.
  • Auth: no endpoints or MCP tools.
  • Credential exposure: every recorded or logged git error goes through _summarize_git_error (userinfo-redacted), and the lease SHA is not secret.
  • Concurrency: the whole cycle is still under _REPO_LOCK and every child goes through run_registered.
  • Argv injection: the branch comes from symbolic-ref, and git rejects --prefixed ref names.
  • Enum completeness: no new status value. refused/diverged are free-text last_error_summary, which sync_health_service passes through verbatim.
  • Docs: git-sync-health.md §1b and agent-lifecycle.md are updated.

Summary

  • Critical: 0
  • Informational: 3 (I1 is worth tidying; I2 needs a release-note line; I3 is a follow-up)
  • Scope: clean
  • Local verification: 79 auto-sync/maintenance/leak tests passed and lint_sys_modules is clean. Live-agent verification against a local dev instance is still to come in a follow-up comment.

🤖 Generated with Claude Code

@dolho

dolho commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Live verification against the local dev instance

Setup: I built the base image from this branch as trinity-agent-base:pr3016, leaving :latest untouched. I created a throwaway agent pr3016-sync through POST /api/agents with base_image: trinity-agent-base:pr3016. Inside that agent container I set up a real repo in /home/developer, a bare origin in /tmp, and a second "foreign" clone that pushes underneath the agent. Each cycle is the container's own agent_server.routers.git._run_auto_sync_once, using the real run_registered and container git 2.39.5.

# Scenario Result
A baseline: local change ✅ success, pushed; behind_after_fetch: 0; last_successful_push_at stamped
B foreign push + local change (the bug) ✅ success; remote log is auto-sync → foreign → auto-sync, linear with no merge commits; behind_after_fetch: 1
C conflicting foreign edit, 3 cycles ✅ each cycle failed, diverged: rebase conflict on main; consecutive_failures 1→2→3; local HEAD the same across all three; no rebase left in progress; the agent's edit intact with a clean status; remote head unchanged
C→backend real 60 s SyncHealthService poll ✅ GET /api/agents/pr3016-sync/git/status returns both new fields in sync_state; agent_sync_state shows failed / 3 / diverged: rebase conflict on main; a sync_failing operator-queue item was raised (priority high, reason in context.last_error_summary)
C-heal after a manual resolution ✅ next cycle success, counters reset
D GIT_SOURCE_MODE=true on main ✅ failed, refused: source-mode on main; no commit made (HEAD unchanged, the file still untracked); remote unchanged
D' same, with an upstream remote (fork-to-own) ✅ success, pushed

Caveats:

  • The agent had no GitHub template, so origin was a local bare repo rather than GitHub. The fetch/rebase/push mechanics are identical, but GitHub auth/transport was not exercised.
  • So the poller would cover the agent, I inserted a local-only agent_git_config row by hand.
  • The cycles were driven directly rather than through the 15-min GIT_SYNC_AUTO loop. The loop only wraps the same function in asyncio.to_thread.

Cleanup: agent deleted; its workspace volume, git-config row, sync-state row, queue item and the :pr3016 image removed.

🤖 Generated with Claude Code

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

merge-train: batch validated on train/20260925-1608 (#3027, full suite green).

@vybe
vybe merged commit d72ee84 into dev Sep 25, 2026
24 checks passed
vybe pushed a commit that referenced this pull request Sep 27, 2026
…efusal helpers beside the settings guard, guard stays after staging

The only git.py conflict was two independent helper blocks added at the same
spot (#3016's _is_missing_remote_ref/_is_shared_source_branch/_rebase_onto_remote
and this PR's _guard_container_only_settings/_has_staged_changes); both kept.
_run_auto_sync_once auto-merged: dev's refusal-before-commit and
fetch/rebase/lease-push are intact, with the guard between `git add -A` and the
staged-only commit check. Docs keep both sides (git-sync-health 1b then 1c;
agent-lifecycle carries #3010/#3011 text plus the ent#708 note). registry.json
rebuilt from the index stages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vybe pushed a commit to harimaruthachalam/trinity that referenced this pull request Sep 29, 2026
…nity-enterprise#705)

Create-time kind: agent (default) | deployment, on POST /api/agents and
MCP create_agent. An explicit source_mode always wins.

An agent gets a working branch + auto_sync_enabled + freeze-on-sync-
failure, but only when the Abilityai#2107 push probe says its token can push to
that repo. A deployment, an ephemeral ghost, a tokenless create, a
refused or unverifiable probe all stay pull-only, with the reason on the
create response's git_mode (never on the /ws broadcast). A template
someone else owns therefore never receives an agent's branches (the
ent#162 class); Cornelius, built from a shared public upstream, is
pinned pull-only. Fork-to-own gets the trio.

Existing agents are not flipped. Operator runbook for migrating a live
agent: docs/migrations/AGENT_WORKING_BRANCH_DEFAULT_2026-09.md.

Stacked on Abilityai#3016, Abilityai#3017, Abilityai#3018 (+Abilityai#3015): merges only after them.

Related to Abilityai/trinity-enterprise#705

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to 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