feat(git-sync): the container pulls origin on its own (trinity-enterprise#703) - #3021
Conversation
…rise#703)
Invariant G3: human and fleet work reaches the agent within a bound.
Nothing inside the container ever pulled; the 2026-09-24 audit found
agents up to 31 commits behind.
- agent server: a pull loop beside the push loop, gated per cycle on
the owner's pull_sync_enabled (read live, GIT_SYNC_PULL fallback),
interval GIT_SYNC_PULL_INTERVAL_SECONDS (defaults to the push one).
Under _REPO_LOCK, never while an execution runs (checked again right
before the tree is touched). Fast-forward, or rebase aborted on
conflict. Uncommitted edits are stashed explicitly and a pull that
collides with them is undone - not --autostash, which strands them
in the stash while reporting success.
- pull_sync_enabled + last_pull_at/_status/behind_after_pull on both
migration tracks; backfill on only where auto-sync is on; new github
agents get it at creation (source mode included), GIT_SYNC_PULL env
derived from the DB flag alone at recreate.
- GET/PUT /api/agents/{name}/git/pull-sync; sync-health persists the
pull outcome (bounded); Settings -> Git sync gains the toggle.
Stacked on #3020 (+#3015-#3018): merges after it.
Related to Abilityai/trinity-enterprise#703
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nto feature/ent703-pull-heartbeat
…eSQL (trinity-enterprise#703)
The inline comment on `pull_sync_enabled` sat between the comma and the
FOREIGN KEY clause. `_PG_TABLE_SUBS` strips that clause only when it directly
follows the comma, so the clause survived without its REFERENCES and
PostgreSQL rejected the table ("syntax error at or near ')'"), failing 18
requires_postgres tests in schema-parity. The comment moves above the column.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
merge-train: ejected from this train because it is stacked on #3020, which is ejected pending a ruling (see #3020). On its own delta this is READY: single Alembic head ( One intent question to settle while it waits: |
|
✅ Nightly unit-suite clean when this PR is merged into |
|
Resolve by merging |
|
✅ Alembic head check clear — merging this PR into Previously flagged; resolved. Advisory — this check does not block merge. · head_sha: |
AndriiPasternak31
left a comment
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Request changes. The design is right: a separate loop, the live flag, an explicit stash instead of --autostash, and push fields kept apart. The real-repo tests are good. I ran them (19/19) plus a few probes against _run_pull_once. Blocking items:
alembic-head-watchis red because of this PR.devnow has0076_operator_queue_ask_objectthrough0079_telegram_group_contextoff0075, and0076_pull_syncstill revises0075, which gives two heads. Rebase, renumber to0080_pull_syncwithdown_revision = "0079_telegram_group_context", and re-resolvedb/migrations.py. The PR is also CONFLICTING in 12 files._integrate_remotecan strand the stash, which is the case the docstring exists to prevent (routers/git.py:876-903).run_registeredraisesTimeoutExpired, so a timed-outmerge --ff-only,stash poporreset --hardskips every restore. Repro: fast-forward times out →failed, the edit is gone from the tree and sits instash@{0}, and the error doesn't mention the stash. Please put atry/finallyaround the post-stash section, and make the error say when a stash entry is left.- The return code of
reset --hardis ignored at:898. If it fails (e.g.index.lockheld by the backend's docker-exec gitignore sweep, which runs outside_REPO_LOCK), the tree is leftUUwith conflict markers. The next auto-sync cycle'sgit add -Acommits them and pushes them to origin with statussuccess. I reproduced this end to end. Please check the return code and stop loudly. I'd also make the push cycle refuse to commit whilediff --diff-filter=Uis non-empty. - The execution gate misses queued turns (
:830).list_running()excludesregister_pendingentries (#2433), so a turn waiting on the chat lock or the headless pool counts as idle. Please countlist_pending_ids()as busy too.
Non-blocking, but worth resolving before merge:
- Where the gate actually protects anyone. For auto-sync agents, the #3011 push cycle already fetches and rebases every cycle with no execution gate. So for the backfilled population the pull is mostly redundant, and the UI line "Never runs while the agent is working" isn't true. Existing source-mode agents, the ones that need the pull, stay off until toggled. Worth saying so in the PR, and probably gating #3011's rebase the same way.
- Pull failures are write-only.
last_pull_errorisn't persisted, there is nolast_successful_pull_at, nothing feedssync_failingor/sync-health, and repo-busy skips aren't recorded. ent#706/#707 will needlast_successful_pull_at, a persistedlast_pull_error, and consecutive pull failure/skip counts. Cheaper to add them to this migration pair than to open a second one on the same table. behind_after_pullis own-branch behind. On atrinity/*agent it reads 0 whilebehind_mainis 1. Please document it.- Docs.
architecture/agent-runtime.md:27still says "two loops". The UI label hard-codes 15 minutes, butGIT_SYNC_PULL_INTERVAL_SECONDSmakes it configurable. The pull cycle also never reaps a staleindex.lock, which matters for pull-only agents.
|
Small correction to item 1 of my review: don't hard-code |
…nto feature/ent703-pull-heartbeat # Conflicts: # src/backend/db/migrations.py # tests/registry.json
…ns, brings main to working branches (trinity-enterprise#703) PR #3021 review: - _with_stash wraps every post-stash step: a git child that times out (run_registered raises TimeoutExpired) goes back to the pre-pull HEAD and re-applies the stash; when that is impossible the error says the edits are kept in `git stash`. - reset --hard's return code is checked. A failed undo stops with an error naming it instead of leaving UU files behind silently. - Both cycles refuse to run over unmerged paths. The push cycle checks before `git add -A`, which would stage conflict markers and push them to origin as a successful sync. - The execution gate counts register_pending entries (#2433), so a turn queued on the chat lock or the headless pool is busy, not idle. - The pull reaps stale lock litter first, as the push cycle does; a pull-only agent has no push cycle to do it. - sync-state gains last_pull_error streak fields: consecutive pull failures / skips and last_successful_pull_at. Ruling on the intent question: a trinity/* working branch only ever pulled itself, so human pushes to main never arrived (G3). The cycle now also merges origin/main (via _get_pull_branch) into the working branch — a merge, not a rebase, since the branch is already pushed; a conflict is aborted and recorded. Nine real-repo tests, all red before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… + UI copy (trinity-enterprise#703) - Alembic 0076_pull_sync -> 0080_pull_sync <- 0079_telegram_group_context (one head, 81 revisions); SQLite entry re-appended after dev's. - The same migration pair adds agent_sync_state.last_pull_error, last_successful_pull_at, consecutive_pull_failures and consecutive_pull_skips, persisted by the sync-health poller (bounded, coerced; a success clears the error). ent#706/#707 read these, so they land here rather than as a second migration on the same table. - agent-runtime.md says three loops; the flow and requirement describe the main merge, the queued-turn gate, the unmerged-path refusal, what behind_after_pull measures, and that auto-sync agents already rebase in the push cycle. - Settings copy no longer hard-codes 15 minutes (GIT_SYNC_PULL_INTERVAL_SECONDS) and no longer claims the push cycle never touches the tree while the agent works. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
All four blocking items, the intent question and most of the non-blocking ones are in. The branch re-merges #3020, which now carries Blocking (Andrii)
Intent (vybe): a Non-blocking
Tests
|
|
Ownership moves to @AndriiPasternak31 (ent#703 handover, per the Mon–Wed plan). The branch is ready for your re-review; I won't push to it further unless you ask. |
AndriiPasternak31
left a comment
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Approve. All four blocking items from my 09-27 review are fixed, each with a real-repo test, and the "bring main into working branches" follow-up is implemented. I found nothing new that blocks. There are three should-fix items below: two are small and cheap to land before merge, the third is at least a wording fix. I reviewed only this PR's own commits (ee8929840..8c1282456, diffed against #3020's head 8b20c1714). #3020 is reviewed separately, and this PR merges after it.
Previous blocking items
- ✅ 1. Alembic head fork.
0080_pull_sync.pynow revises0079_telegram_group_context, and the SQLitepull_syncentry comes afterdev's.check_alembic_heads.pyreports 81 revisions and 1 head (0080_pull_sync).alembic-head-watchis green, and the PR is MERGEABLE. #2984 and #3005 both carry an0080_*on0079and are both mergeable, so whichever of the three lands later has to re-chain. Re-run the head check right before merge. - ✅ 2. Stash stranded on timeout. Every step after the stash now runs inside
_with_stash(routers/git.py:891-957). OnTimeoutExpiredit resets topre_headand pops. If that can't be done, the error ends with "local edits are kept ingit stash". Tests:test_a_timed_out_fast_forward_puts_the_edits_backandtest_a_stash_left_behind_is_named_in_the_error. - ✅ 3.
reset --hardreturn code ignored / markers pushed. The return code is checked now: a failed reset returns "…could not be undone…". Both cycles refuse to run while_unmerged_pathsis non-empty, and the push cycle checks beforegit add -A(:1159). My end-to-end repro is nowtest_a_failed_undo_stops_loudly_and_the_push_refuses_the_markers: the push returnsrefused: unmerged paths (notes.md),origin/mainis unchanged, and no markers reach it. - ✅ 4. Queued turns counted as idle.
_executions_in_flightreturnslist_running()+list_pending_ids()(:830-845). Test:test_a_queued_turn_counts_as_busy. - Non-blocking items from last time:
- Done: pull health persisted (
last_pull_error,last_successful_pull_at, pull failure/skip streaks), stale-lock reaping before the pull,behind_after_pulldocumented, "three loops" inagent-runtime.md, and the UI copy no longer hard-codes 15 minutes. - Deferred with a stated reason: execution-gating #3011's push-cycle rebase. That's fine as its own change.
- Done: pull health persisted (
- ✅ "Pull
maininto working branches."_integrate_source(:975-994) mergesorigin/<main>into atrinity/*branch every cycle and aborts on conflict. Tests:test_main_is_merged_into_the_working_branchandtest_a_conflicting_main_is_aborted_and_recorded. The PR body still lists this as out of scope, so please update it.
New findings
-
[should-fix] A conflict merging
mainis recorded as "Auto-merging <file>".docker/base-image/agent_server/routers/git.py:991- Problem: on a conflict, git prints to stdout and leaves stderr empty.
_summarize_git_error(merge.stderr or merge.stdout)therefore takes stdout's first line, which is the auto-merge notice, not the conflict. - Reproduced on a real repo: the agent commits
notes.mdon its working branch, a human commitsnotes.mdonmain, and_run_pull_oncereturns{'status': 'failed', 'error': 'merging main: Auto-merging notes.md'}. That exact string is also persisted aslast_pull_error. - Failure scenario: every 15 minutes the same failure repeats with an error that reads like progress. ent#706/#707 will surface this field directly, and an operator can't tell it is a conflict.
- Also here: the
merge --abortreturn code is ignored (:990). The unmerged-paths guard does stop the push, but the error doesn't say the abort failed. - The current test only asserts
startswith("merging main: "), so it doesn't catch this. - Fix: mirror
_rebase_onto_remote. Read--diff-filter=U(or look forCONFLICTin the output) before aborting, and returndiverged: merge conflict with main (notes.md). Check the abort's return code and say so if it failed. Make the test assert the conflict wording.
- Problem: on a conflict, git prints to stdout and leaves stderr empty.
-
[should-fix] Any rebase of a working branch silently flattens the merge commit the pull just made.
routers/git.py:803(git rebase --autostash origin/<branch>), reached from_integrate_remote(:970) and the push cycle (:1205)- Problem: plain
git rebasedrops merge commits. It replaysmain's commits one by one onto the working branch as new SHAs. - Reproduced: auto-sync is off, cycle 1 merges
mainintotrinity/a/1(1 merge commit, not pushed), then a human pushes toorigin/trinity/a/1. Cycle 2 returnssuccess, butgit log --mergesis empty,main's commit reappears as a copy, andorigin/mainis no longer an ancestor of HEAD. - Failure scenario:
behind_mainkeeps reporting the agent as behind even though the content is there. The next cycle mergesmainagain on top of the duplicates, and the history the agent pushes carries copied commits. - The PR's own reasoning for merging rather than rebasing ("rewriting it would make the next push cycle rebase it back") is exactly what this undoes.
- Rare today: it needs an unpushed merge plus someone else pushing to the agent's own branch. It gets likelier once pull stays on while auto-sync is toggled off.
- Fix: use
rebase --rebase-mergesin_rebase_onto_remote, or merge instead of rebasing when the branch istrinity/*. Add a test for this sequence.
- Problem: plain
-
[should-fix] "Never while a turn runs" is check-then-act.
routers/git.py:1072-1082- Problem: the gate is read before
_integrate_remote/_integrate_sourcerun, but nothing on the admission side waits for it.register_pendinginchat.py:53/204,claude_code.py:750andresult_callback.py:418never looks at_REPO_LOCKor at a pull in progress. - Failure scenario: a turn admitted during the integrate window (a fast-forward or merge of up to 60 s, a rebase up to 120 s, plus the stash operations) reads files while HEAD is moving. That is the case the issue's acceptance criterion rules out ("serialize on
_REPO_LOCKand the execution lock"). - The claim appears as an absolute in three places: the Settings copy (
GitSyncSettingsPanel.vue:75, "never runs while the agent is working"), requirement §11.18 andagent-lifecycle.md. - Fix: hold admission briefly while a pull is integrating (for example, a pull-in-progress event that
register_pendingcallers wait on, with a bound). Or, at minimum, change the copy to "never starts while…" and document the window.
- Problem: the gate is read before
-
[nit] A failed ahead/behind count reads as "up to date."
routers/git.py:1065-1070_compute_ahead_behindreturns(0, 0)on any failure, including its 10 s timeout. The pull then recordssuccesswithbehind_after_pull=0and moveslast_successful_pull_atforward.- Fix: give the pull cycle a strict variant that fails the pull instead.
-
[nit] A timed-out step with nothing stashed is left as the kill left it.
routers/git.py:946-957- The reset back to
pre_headonly happens whenstashedis true. - Failure scenario: a killed
merge --ff-onlyormergeon a clean tree can leave a half-updated tree or aMERGE_HEAD. The next push cycle can then commit that partial state as the agent's own work. - Fix: reset to
pre_headon timeout whether or not anything was stashed. If anindex.lockfrom the killed child blocks the reset, the error should say so.
- The reset back to
-
[nit] The PR description is stale.
- "Out of scope: merging the source branch into a working branch" is now in scope.
- The test count still says 19; there are 28 now.
- "Live on local dev" is still unchecked. The loop's startup wiring (
schedule_auto_sync_if_enabled) and the real process-registry import have only run under unit tests, never inside an agent container.
What I checked
- Read this PR's own diff against #3020's head. That covers the agent server (
auto_sync.py,routers/git.py), both migration tracks and the schema, thepull-syncroutes, creation/recreate env (crud.pyandlifecycle.py), the sync-health poller, the UI panel and the docs. - Architecture checks:
- Creation and recreate don't touch the PAT gate or the push blackhole; the pull only fetches.
- Errors pass through
_summarize_git_error, which redacts URL userinfo. - The pull never writes the push's
consecutive_failures, so the freeze still keys on push health. The newrefused: unmerged pathscounts toward the freeze, which is the right direction. - The reaper reuses the #1595 1-hour age gate. It never removes a live
index.lock. - Shutdown cancels both loop tasks.
- Tests: 16 touched and related unit files (pull cycle, #3010, #3011, auto-sync, #1595, #2742, sync health/state, ent109 env seam, #1484, fork-to-own, pull branch, dual ahead/behind). 429 passed + 1 skipped on seed 12345, 430 passed on seed 99999.
- Real-repo probes (bare origin, agent clone, human clone) for findings 1 and 2.
check_alembic_heads.py: 1 head.gh pr checks 3021: every check passing or skipped, none failing.
#3005 landed Alembic 0080_agent_skill_sets on 0079_telegram_group_context, the same parent this branch's 0080_pull_sync used, so merging as-is would leave two heads and `upgrade head` would apply nothing. Renumber the revision to 0081_pull_sync, chained on 0080_agent_skill_sets (file, revision, down_revision, docstring, the SQLite mirror note and the revision test). SQLite MIGRATIONS keeps dev's agent_skill_sets entry first, pull_sync after it. tests/registry.json keeps both entries. No logic change.
|
Merged
Merge order: #3035 adds @vybe could you review? I approved this earlier today, but I've pushed to it since, so it needs an approver other than me (SOC 2). |
Dismissing my own approval: I also pushed commits to this branch (the dev merge fixing the 0080 migration fork), so this approval would cover my own code (SOC 2 separation of duties). Needs an independent review.
…nto feature/ent703-pull-heartbeat # Conflicts: # tests/unit/test_1484_create_agent_characterization.py
…p merges (trinity-enterprise#703) PR #3021 re-review: - A conflict merging main was recorded as "Auto-merging <file>": git prints it on stdout with an empty stderr. Unmerged paths are read before the abort; the error is "diverged: merge conflict with main (<files>)", and a failed merge --abort is named. - `git rebase` flattened the merge the pull made into copies of main's commits; both cycles now rebase with --rebase-merges. - A failed strict ahead/behind count fails the pull instead of reading as up to date. - A timed-out step is reset to the pre-pull HEAD even with nothing stashed; a reset blocked by the killed child's index.lock says so. - "Never runs while the agent is working" is check-then-act: the copy and docs now say "never starts" and document the admission window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the 09-28 re-review:
Verification
🤖 Generated with Claude Code |
obasilakis
left a comment
obasilakis
left a comment
There was a problem hiding this comment.
Approve on content.
- Security: the new endpoints use the same auth as auto-sync (reading the flag needs agent access, changing it needs ownership). The pull refuses to run over unresolved conflicts, and the push refuses to commit them. Status text written by the agent is limited to the known values and truncated before it is stored.
- Both migration tracks are present. Merged into dev, the Alembic history has a single head (
0081_pull_sync). - The PR's tests pass locally (178). Two deliberate code breaks were each caught by a test: removing the push cycle's refusal to commit unresolved conflicts, and dropping
--rebase-merges.
Follow-ups (non-blocking):
- File the issue for the window between the idle check and the pull: a turn admitted during a pull can see files change mid-read.
- Add backend tests for
GET/PUT .../git/pull-sync: a non-owner is rejected, and 404 when git is not configured. - The
# mcp:header inrouters/git.pydoes not mention pull-sync. - Release notes: agents with auto-sync already on get pull turned on at upgrade and will merge
maininto their working branch every cycle.
Merge after #3020. tests/registry.json will conflict once #3020 lands; resolving it is a small by-hand merge.
…eartbeat Brings the updated base (with current dev). Resolve: registry = base file plus the ent#703 entry; agent-lifecycle keeps the pull-cycle paragraph beside the base's updated agent-side paragraph; git-sync-health keeps both new sections (1c ent#708, 1d ent#703). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/ent703-pull-heartbeat dev gained 0081_portal_messages_unread_idx (#3076). Resolve: - db/migrations.py: keep dev's portal_messages_unread_index entry, then pull_sync. - Alembic: 0081_pull_sync -> 0082_pull_sync, down_revision 0081_portal_messages_unread_idx, so the version line keeps a single head. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
merge-train: not on this train — rides the next train once fixed. Race in the pull cycle: the "no execution running" check happens before the pull, but nothing holds turn admission during it. Also noted, not blocking: Heads-up for sequencing: #3035 also adds |
|
Migration clash with current This branch adds Fix before merge:
#3022 carries this migration and will need a re-merge of #3021 after that. |
… revision to 0083 dev gained 0082_agent_sync_state_divergence (#3035), which adds its own agent_sync_state columns. Union both column sets across schema/tables/ sync_state/sync_health_service, chain 0083_pull_sync off dev's head, and order the SQLite entry after agent_sync_state_divergence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#2996's route census (merged to dev) requires every new route to be classified. The agent's pull loop reads GET .../git/pull-sync with its own key each cycle (AGENT_CALLABLE); the PUT is a setting write, so it takes Depends(require_person) like the other #2996 settings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
merge-train: not on this train. It rides the next one once fixed. The migration renumber is right: Yesterday's ejection reason is still open. The two commits since then (
Also noted, not blocking:
|
dev moved past 0082 (0083 to 0085 landed), so 0083_pull_sync forked the Alembic graph into two heads, which alembic-head-watch flagged. It is now 0086_pull_sync on 0085_ent720_email_identity, and the SQLite list keeps both sides, with dev's entries first and pull_sync after. The merge also brings #3107, the agent-server boot fix whose absence failed journey-smoke. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dev moved past 0082 (0083 to 0085 landed), so 0083_seat_ask_class_state forked the Alembic graph into two heads, which alembic-head-watch flagged. It is now 0087_seat_ask_class_state on 0085_ent720_email_identity. 0087 rather than 0086 because #3021's pull_sync takes 0086; whichever of the two merges second re-parents onto the other. The SQLite list keeps both sides, with dev's entries first. The merge also brings #3107, the agent-server boot fix whose absence failed journey-smoke. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vybe
added
the
status-needs-fix
|
merge-train: not on today's train. I've labelled this |
…estatement (#3021) dev gained 0086_metric_points_restatement off 0085, so this branch's 0086_pull_sync was a second head and alembic-head-watch failed. Renamed to 0087_pull_sync and re-parented; the SQLite list keeps dev's entry first, matching the Alembic order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
removed
the
status-needs-fix
dolho
left a comment
dolho
left a comment
There was a problem hiding this comment.
Review — CI fixed; solid, one timing issue worth fixing
CI: alembic-head-watch was red: dev gained 0086_metric_points_restatement off 0085, the same parent as this branch's 0086_pull_sync → two heads. Merged dev, renamed the revision 0087_pull_sync and re-parented it onto 0086_metric_points_restatement; the SQLite list keeps dev's entry first, matching the Alembic order; test_ent703_pull_cycle.py's revision pin follows (2d519feaf). Locally: check_alembic_heads → 1 head, check_alembic_parity PASS, 193 related tests pass; submodule pointers equal dev's. #3022 (stacked) has the fix merged in.
⚠️ Merge order with #2984: both now add a revision directly off0086_metric_points_restatement; whichever lands second needs a one-line re-parent (alembic-head-watchwill flag it).
Overall: solid and defensive — an explicit stash instead of autostash, abort on any conflict, --rebase-merges keeps the main merge, refuses over unmerged paths. The PUT is OwnedAgentByName + require_person; the GET is the agent-readable auto-sync shape; both migration tracks present (IF NOT EXISTS, same backfill). No new backend loop, so no leader lease needed. test_ent703_pull_cycle.py + test_sync_health_service.py (83) pass.
Medium — the push and pull loops start together and silently skip each other (verified)
agent_server/auto_sync.py: the pull interval defaults to the push interval (get_pull_interval_seconds, L211–218), both loops start at boot and sleep the same 900 s (L154/161 and the pull loop), and both take _REPO_LOCK non-blocking (routers/git.py:1203, 1308) → the loser returns skipped/repo_busy. The pull records nothing for that skip and the push only logs it, so the two can keep colliding or take turns: the effective pull bound (and the push cadence on existing agents) can quietly double with nothing in sync-state to show it. Before this PR the push loop's only competitor was an occasional operator call.
Fix: offset the pull loop by half an interval or add jitter; better, have the two loops wait briefly for the lock (acquire(timeout=…)) while operator endpoints keep the 409; or at least record a repo_busy skip so it feeds consecutive_pull_skips.
Low — reset --hard pre_head can drop writes made during the pull
_with_stash (routers/git.py:1093, 1111): on the stash-pop-conflict and timeout paths, reset --hard discards tracked-file writes made after the stash — a turn admitted in that window, files-API writes, and the backend's docker-exec git ops, which run outside _REPO_LOCK (see the comment at routers/git.py:555). The window is seconds, but the loss is silent. Re-check _executions_in_flight() right before the reset and record a failure instead when busy, or narrow the reset to the paths the pull touched (git checkout pre_head -- <files in pre_head..HEAD>).
Nits
routers/git.py:1157— whengit mergerefuses up front (e.g. an untracked file would be overwritten) there is no MERGE_HEAD, so the unconditionalmerge --abortappends a spurious "; merge --abort failed (There is no merge to abort)". Abort only when.git/MERGE_HEADexists.git stash popwithout--indexbrings staged edits back unstaged — harmless today (the push cycle runsgit add -A) but a tree-state change a "never discards work" pull may be assumed not to make.- A permanently busy agent records
skippedevery cycle and never pulls;consecutive_pull_skipsis stored but nothing in sync health alerts on it. A threshold like the push-failure surface would close it. - Running source-mode agents stay pull-off after upgrade (the backfill only follows existing auto-sync). Matches the ruling — worth one line in the PR / release notes so ops toggles it deliberately.
🤖 Generated with Claude Code
vybe
added
the
status-needs-fix
|
merge-train: not on today's train. It rides the next one once fixed. The push that cleared I've set |
…tarving each other (#3021 review) The two background loops started together on one interval and took _REPO_LOCK non-blocking, so the loser skipped silently every tick and the effective pull bound could quietly double. Background cycles now wait up to GIT_SYNC_LOCK_WAIT_SECONDS (default 120 s) for the lock, and the pull loop's first tick is half an interval after the push loop's. Operator endpoints keep their immediate 409. The pull's reset --hard undo re-checks that no execution started meanwhile; if one did, the tree is left as it is (conflict markers block the next push and pull) instead of discarding the turn's writes. merge --abort runs only when MERGE_HEAD exists, so a merge that refused to start no longer records "There is no merge to abort". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
removed
the
status-needs-fix
|
Review follow-up —
Tests: new 🤖 Generated with Claude Code |
…afe_to_reset guarantees (#3021) — mechanical, per the merge-train note on the PR The undo paths no longer reset over a registered execution, but writes outside the process registry (Files API, docker exec, web terminal) made during the integrate window are not protected, as the author's e6cb8b5 note concedes. Wording only: two git.py docstrings, the auto_sync.py comment, requirements/github.md and the GitSyncSettingsPanel.vue copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
merge-train (2026-10-02): on this train. I pushed one wording-only commit to this branch:
Locally: Non-blocking follow-ups:
|
vybe
left a comment
vybe
left a comment
There was a problem hiding this comment.
merge-train (2026-10-02): validated at lane C+schema. The ejection finding is closed in code by _safe_to_reset, the claims are narrowed in 96dddec00, one Alembic head (0087_pull_sync), Tier 1 and Tier 2 green on this head.
5 participants
Summary
Invariant G3: human and fleet work reaches the agent within a bound. Nothing inside the container ever pulled, and the 2026-09-24 fleet audit found agents up to 31 commits behind.
pull_sync_enabled. The flag is read live fromGET .../git/pull-syncwith the agent's own key, withGIT_SYNC_PULLas the fallback (the bug(git-sync): the auto-sync toggle is not authoritative — OFF never takes effect (the baked env is OR'd forward at recreate), ON waits for the next recreate #3010 pattern). IntervalGIT_SYNC_PULL_INTERVAL_SECONDS, defaulting to the push interval (900 s)._run_pull_once, under_REPO_LOCK):list_running()+list_pending_ids()). Checked from the process registry, and again right before the tree is touched; an unreadable registry counts as busy. This is check-then-act: turn admission does not wait on a pull, so a turn admitted during the integrate window can see HEAD move (documented; holding admission is a follow-up).--rebase-merges), aborted on conflict.trinity/*working branch also mergesorigin/mainin (_integrate_source), so human pushes tomainarrive. A conflict is aborted and recorded asdiverged: merge conflict with main (<files>).--autostash. When incoming commits touch the same files, autostash's re-apply conflicts, leaves the edits only in the stash, and still reports success. Here a colliding pull is undone (back to the pre-pull HEAD, where the stash applies cleanly) and recorded._safe_to_reset). Writes made outside the process registry (Files API, docker exec, web terminal) during the integrate window can be lost if a conflicting pull is undone.pull_sync_enabledon both migration tracksauto_sync_enabledis already ongithub:agents get it at creation, source mode included (pull-only agents are the ones that most need it); ghosts excludedGIT_SYNC_PULLderived from the DB flag alone at recreatelast_pull_at/last_pull_status/last_pull_error/behind_after_pull/last_successful_pull_at/consecutive_pull_failures/consecutive_pull_skipsinsync-state.json, persisted onagent_sync_state(status bounded to its vocabulary, counter coerced). A failed pull never touches the push'sconsecutive_failures.maininto a working branch, every cycle.sync_failing//sync-health(ent#706/bug: workspace canvas panel flickers + charts broken in update_panel #707)Changes
auto_sync.py:run_pull_loop/run_one_pull_cycle/resolve_pull_sync_enabled; the live flag read is generalised (_resolve_flag) and shared with the push looprouters/git.py:_run_pull_once,_with_stash,_integrate_remote,_integrate_source,_unmerged_paths,_executions_in_flight,_record_pull;_rebase_onto_remoteuses--rebase-merges;/api/git/statusreportspull_sync_enabledpull_sync+ Alembic0081_pull_sync←0080_agent_skill_setsdb/schedules/git_config.py,db/sync_state.py,db_models.py,database.pyrouters/git.py+models.PullSyncToggle:GET/PUT .../git/pull-synccrud.py: on at creation + envlifecycle.py: env from the DB flagsync_health_service.py: persists the pull outcomeGitSyncSettingsPanel.vue,stores/agents.jstest_ent703_pull_cycle.py(33, real repos)test_1484(+2: creation turns pull on for agents and deployments)test_ent109_git_env_seam(+2: env follows the DB flag alone; the owned-key guard fixture now writes every owned var)test_sync_health_service(+2: persisted, and agent-written garbage dropped)gitSyncSettingsPanel.spec.js(+2, mounted)requirements/github.md§11.18feature-flows/git-sync-health.md§1darchitecture/agent-lifecycle.mdarchitecture/api-endpoints.mdTest Plan
148462,ent10924,sync_health_service49vitest run3632/3632 (ratchets included);check:tokensOK_integrate_remotewith a naiverebase --autostashfails exactly the colliding-edits testlint_sys_modules, root placement, enterprise-docs guard clean; single Alembic head (0081_pull_sync)Related to abilityai/trinity-enterprise#703 (cross-repo; close at release)
🤖 Generated with Claude Code