Skip to content

fix(sync): scope diff/push to one branch tree and report orphans (#649) - #654

Merged
padak merged 1 commit into
mainfrom
fix/649-branch-scoped-diff-orphans
Aug 22, 2026
Merged

fix(sync): scope diff/push to one branch tree and report orphans (#649)#654
padak merged 1 commit into
mainfrom
fix/649-branch-scoped-diff-orphans

Conversation

@padak

@padak padak commented Aug 22, 2026

Copy link
Copy Markdown
Member

The bug

After sync pull --branch <dev> re-targets manifest.configurations to the dev branch, the default main/ tree is orphaned on disk. A subsequent production sync diff / sync push then classified every config in main/ as added with an empty config_id, and every dev-only config as added with an id — one sync push away from duplicating an entire production project. Verified live on project 5946 (v0.88.0): added: 19 where the correct answer is 0.

No scaffolding or hand-edited manifest is needed to hit it; the documented multi-branch pull is enough. This is the same failure class as #482, on the axis #482's fix does not cover: #482 scoped the untracked-file scan to the source branch subtree, which protects a dev-branch push from orphaned trees. A production push reads main/ as its legitimate source — and main/ is exactly the tree pull --branch just orphaned.

Root cause — two independent legs

  1. Untracked-scan leg. The adopt-by-id guard (sync push creates duplicate configs for every config a dev branch inherits from main (even with zero local changes) #482) refused adoption whenever the id was claimed by ANY manifest entry (untracked_key not in tracked_keys). tracked_keys is keyed "{component_id}/{config_id}" with no branch dimension, so after the re-target every id in main/ looked like a fork-by-copy and the create was kept.
  2. Tracked-entry leg. diff() built the local side from all manifest.configurations regardless of the target branch, so configs tracked on the dev branch were compared against the production remote.

The fix

Both legs are fixed inside the diff computation, which push consumes — so sync push and sync push --dry-run inherit the classification unchanged.

  • The local side is scoped to the ONE tree the diff reads (the source branch path: the target branch's own subtree, or main/ when the target has none — the KFR-07 promote path). Manifest entries whose branch resolves to a different tree are excluded from the changeset and reported under a new orphaned bucket. This also closes a latent inconsistency: _sync_push_ops.push_update already reads every file through the source path, so a cross-tree entry was diffed from one file and pushed from another (or failed with FileNotFoundError).
  • The id-claim check is branch-aware. A claim by an entry in the same tree keeps today's fork-by-copy CREATE (the sync push creates duplicate configs for every config a dev branch inherits from main (even with zero local changes) #482/sync push: write adopted-by-id configs back into the manifest (follow-up to #482) #497 contract). A claim held only by another tree means the manifest was re-targeted: the file is adopted when its id still resolves on the target branch (diffed as unchanged/modified), and reported as stale_branch_tree when it does not — never re-created.
  • Branch identity is normalized through the branch tree path, because production is spelled three ways in real manifests: None (CLI), 0 (branchId=branch_id or 0 from a git-branching pull) and the default branch's numeric id (plain pull). All three name the same tree.
  • orphaned is additive (--json): a summary.orphaned counter plus a details list carrying component_id, config_id, path, branch_id, branch_path, exists_on_target, reason (tracked_on_other_branch / stale_branch_tree) and a hint. Human mode prints a warning naming the fix (sync pull to re-target, branch merge to promote a dev-only config) and previews the first 10 rows — a re-targeted manifest orphans a whole project at once.

Acceptance from the issue holds: the production diff after a dev pull reports added: 0, the pre-existing configs as unchanged, plus warnings; sync push --dry-run plans no creates.

One deviation from the issue's sketch

The issue proposed gating a cross-branch tracked entry on "does its id resolve on the target remote". Implemented as written, that entry and the adopted main/ file for the same config would both enter local_configs — the config would be counted twice, with contradictory classifications. The code also proves the entry cannot be pushed correctly anyway (push_update reads the source tree, not the entry's own tree). So the participation rule is the source tree instead, which is strictly narrower: the KFR-07 "promote the default tree to a dev branch" flow never trips it (there the entries' tree is the source tree, so promote-CREATE still works — covered by a new test), and remote resolution is kept as the signal that words the orphan hint and drives adoption.

Structure

The branch-scoping logic lives in a new src/keboola_agent_cli/sync/branch_scope.py; the two untracked-file walkers moved there with it and sync_service.py now delegates. It sits alongside the sync/branch_registry.py extraction that landed with #644 — the two modules are disjoint concerns (which tree a diff reads vs. registering the directory a scaffold is written to) and both delegates coexist in sync_service.py. That file is grandfathered at 1655 code lines in scripts/file_size_baseline.json and may only shrink, so the extraction is required, not cosmetic: main sits at 1653, this branch brings it to 1604.

Tests

13 new, TDD (they reproduce added == 2 against the pre-fix code):

  • TestIssue649ProductionDiffAfterBranchPull (service level, tests/test_sync_service.py): the issue's repro for diff / push / push --dry-run, the orphaned bucket's contents and hints, a stale-tree file whose id no longer resolves, the dev-branch control (unchanged behaviour), the KFR-07 promote-still-creates guard, a healthy single-branch tree (bucket stays empty), a legacy branchId: 0 manifest, a unit test for the branch normalizer, and the interaction with config new --output-dir --push writes a scaffold without the created config ID → duplicates on next sync push #644 (a config new --push --output-dir scaffold placed in a dev-branch subtree is still adopted by id, never re-created).
  • TestSyncOrphanWarningCli (tests/test_sync_cli.py): human mode warns instead of printing "No differences found", and caps the list.

Existing #482 / #497 / #472 regression tests stay green. Rebased onto main after #650/#644 landed; full suite: 6006 passed, 61 skipped. make lint format-check typecheck skill-check version-check version-gate-check command-sync-check changelog-check check-error-codes check-sentinel-guards loc-check all green (the one ty diagnostic is the pre-existing hatch_build.py import).

No version bump and no changelog.py entry (per the release-PR convention); the new gotcha is tagged (since vNEXT).

Docs synced: sync-workflow.md, branch-workflow.md, gotchas.md, commands-reference.md, AGENT_CONTEXT (context.py).

Fixes #649


Open in Devin Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

A production `sync diff` / `sync push` run after `sync pull --branch <dev>`
classified every config in the orphaned `main/` tree as `added` with an empty
`config_id`, and every dev-only config as `added` with an id -- one push away
from duplicating an entire production project. No scaffolding was needed; the
documented multi-branch pull was enough.

Root cause, two independent legs:

1. The adopt-by-id guard (#482) refused adoption whenever the id was claimed by
   ANY manifest entry. `tracked_keys` has no branch dimension, so after a pull
   re-targeted the manifest to the dev branch, every id in `main/` looked like a
   fork-by-copy and kept the create.
2. `diff()` built the local side from ALL `manifest.configurations`, so entries
   tracked on the dev branch were compared against the production remote.

Both are fixed in the diff computation, which push consumes (so `push` and
`push --dry-run` inherit them):

- The local side is now scoped to the ONE tree the diff reads -- the source
  branch path. Entries whose branch resolves to a different tree are excluded
  and reported under a new additive `orphaned` bucket (`summary.orphaned` +
  per-entry details and hints; human mode previews the first 10). This also
  removes a latent inconsistency: `push_update` already reads every file through
  the source path, so a cross-tree entry was diffed from one file and pushed
  from another.
- The id-claim check is branch-aware. A same-tree claim keeps today's
  fork-by-copy CREATE (#482/#497 contract intact). A claim only from another
  tree means the manifest was re-targeted: the file is adopted when its id still
  resolves on the target branch, and reported as `stale_branch_tree` when it
  does not.
- Branch identity is normalized through the branch tree path, so production
  spelled `None`, `0` (git-branching pull) or the default branch's numeric id
  all name the same tree.

The branch-scoping logic lives in a new `sync/branch_scope.py` (the untracked
walkers moved with it), alongside the `sync/branch_registry.py` extraction from
#644; `sync_service.py` is grandfathered at its size and may only shrink.

Tests: 13 new (11 service-level covering the issue's repro, the KFR-07 promote
flow, the dev-branch control, legacy `branchId: 0`, the normalizer, and the
#644 scaffold-adoption interaction; 2 CLI covering the human warning and its
preview cap). Full suite green.

Docs: sync-workflow.md, branch-workflow.md, gotchas.md, commands-reference.md,
AGENT_CONTEXT.
@padak
padak force-pushed the fix/649-branch-scoped-diff-orphans branch from bd043ed to 7cde8aa Compare August 22, 2026 22:46
@padak
padak merged commit 63475d4 into main Aug 22, 2026
4 checks passed
@padak
padak deleted the fix/649-branch-scoped-diff-orphans branch August 22, 2026 22:58
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.

sync: production diff/push after 'sync pull --branch' flags the whole orphaned main/ tree as added (mass-duplicate risk)

1 participant