docs: Update orchestrator worktree sync for bidirectional sync [doc-updater] - #856
Conversation
Update the 'Worktree state synchronization' section to reflect bidirectional sync after the changes in PR #855 added fetch-from-remote functionality. The docs previously only described push operations (orchestrator -> remote). Now they cover both directions: - Push to remote (existing): contract init, phase completion, failure - Fetch from remote (new): before phase execution to handle orchestrator restarts This completes the documentation for the worktree sync mechanism that prevents ContractNotFoundError after orchestrator restarts when the local worktree branch lags behind origin. Triggered by: e121e26 (PR #855) Authored-by: egg
There was a problem hiding this comment.
No agent-mode design concerns. This is a documentation-only change updating internal orchestrator architecture docs to reflect the bidirectional worktree sync mechanism. It doesn't affect agent workflows, prompts, or output formats.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Documentation-only change. The restructured section accurately describes the bidirectional sync mechanism from PR #855. One inaccuracy to flag:
Minor inaccuracy in divergence description (line 123 of orchestrator.md)
The doc says the function "skips gracefully when...local has diverged from remote (uncommitted local work)." The parenthetical is misleading. The actual code (_sync_worktree_with_remote at orchestrator/routes/pipelines.py:1694-1713) checks git rev-list --left-right --count HEAD...origin/<branch> for committed local work ahead of remote — not uncommitted changes. In fact, git reset --hard (the next step) would discard uncommitted changes.
Suggested fix — change:
it skips gracefully when the remote branch doesn't yet exist (first pipeline run), when fetch fails, or when local has diverged from remote (uncommitted local work).
to:
it skips gracefully when the remote branch doesn't yet exist (first pipeline run), when fetch fails, or when the local branch has commits not on the remote (local-ahead divergence).
This matches the code comment at line 1708: "Local branch has commits not on remote — skipping reset".
Not blocking since this is docs only, but worth fixing to avoid confusing future readers about what kind of divergence is detected.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Agreed. Fixed the inaccurate parenthetical — changed "uncommitted local work" to "local-ahead divergence" to match the actual git rev-list --left-right --count check in _sync_worktree_with_remote(). The new wording aligns with the code comment at line 1708.
Pushed in 627d4f3.
— Authored by egg
There was a problem hiding this comment.
No agent-mode design concerns. The only change since the last review is a documentation wording fix addressing prior feedback — correcting "uncommitted local work" to "local-ahead divergence" to accurately describe the git rev-list check. No agent-mode impact.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review (delta from a2bc41b to 627d4f3)
Single commit 627d4f3 addresses the feedback from the previous review. The change replaces "(uncommitted local work)" with "(local-ahead divergence)" in the inbound sync description.
Verified against source: The new wording matches _sync_worktree_with_remote() at orchestrator/routes/pipelines.py:1694-1713, which uses git rev-list --left-right --count HEAD...origin/<branch> to detect committed local-ahead divergence (not uncommitted changes). The log message at line 1708 also uses the same "commits not on remote" phrasing now reflected in the docs.
No issues found. Previous feedback fully addressed.
— Authored by egg
|
egg review completed. View run logs 4 previous review(s) hidden. |
docs: Update orchestrator worktree sync for bidirectional sync [doc-updater]
Update the "Worktree state synchronization" section in the orchestrator architecture doc to reflect the bidirectional sync mechanism added in PR #855.
Changes:
_sync_worktree_with_remote()function that handles orchestrator restart scenariosContext:
The previous documentation only described the push-to-remote behavior (orchestrator writes artifacts, pushes to remote). PR #855 added the complementary fetch-from-remote operation that runs before pipeline phase execution to ensure the local worktree is in sync with commits pushed by agents in previous phases. This prevents
ContractNotFoundErrorafter orchestrator restarts.Triggered by: e121e26 (PR #855 "Sync worktree with remote before pipeline phase execution")
Authored-by: egg