fix(sync): close the sync cycle with per-resource pull reconciliation#2853
fix(sync): close the sync cycle with per-resource pull reconciliation#2853daryllimyt wants to merge 6 commits into
Conversation
daryllimyt
added
fix
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a25c0fdd64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
✅ No security or compliance issues detected. Reviewed everything up to ee74c72. Security Overview
Detected Code Changes| Change Type | Relevant files ... (code changes summary truncated to fit VCS comment limits.) |
daryllimyt
force-pushed
the
feat/revamp-git-sync-2
branch
from
a25c0fd to
7379e69
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7379e6982c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Pull now blocks only on per-resource conflicts (local AND remote changed with differing specs) instead of any local drift, and rebaselines convergent resources so export -> merge -> pull ends clean (ENG-1472) - Track resource hashes in two spaces: last_synced_spec_hash (repo spec) and last_projected_spec_hash (local projection), recorded from a post-reconcile re-projection that doubles as the P(W_next) == S postcondition check (ENG-1475) - Stop advancing mapping sync metadata on branch export; it only moves on pull when the base advances (ENG-1473) - Apply mapping-style source id dedup in read-only projections so duplicate slugs cannot shadow workflows (ENG-1474) - Drop the unwritten WorkspaceSyncEvent provenance table from the v1 migration (journal ships post-v1, ENG-1477) and fold rendered_files into the base migration; reparent onto 9b52f7f18a31
- read_files fetches only the manifest and canonical workflow definition paths, via blob SHAs, and skips non-UTF-8 blobs instead of crashing - Strip and require a non-empty commit message in write_files - Fetch commit/branch pages off the event loop (PaginatedList iteration triggers lazy page requests) - Add is_workflow_definition_path predicate; non-canonical paths under the workflow root are ignored rather than parsed - Normalize slug inputs: whitespace-only aliases fall back to the title before the generic 'workflow' fallback
- Pending changes now include non-exportable DELETE entries for synced resources missing from the local projection, so status and counts stay honest - Introduce PullReconciliationPlan shared by pull and status: convergent resources (equal specs or deleted on both sides) rebaseline; resources deleted remotely but unchanged locally are untracked back into pending creates; resources deleted locally but changed remotely are re-imported (Git owns desired state); only divergent dual edits conflict - parse_files only parses canonical workflow definition paths
- Strip and reject whitespace-only ChangeSetCreate.title and ChangeSetExport.message - Validate pr_base_branch when provided (empty string no longer silently falls back to the default base branch) - Add a random suffix to legacy auto-generated publish branch names to avoid same-second collisions
The unique index on id already enforces uniqueness; the extra UNIQUE constraint created a second identical index on every sync table.
daryllimyt
force-pushed
the
feat/revamp-git-sync-2
branch
from
7379e69 to
ee74c72
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee74c72335
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> bool: | ||
| """Return True for canonical `<root>/<source-id>/definition.yml` paths.""" | ||
| parts = path.strip("/").split("/") | ||
| return ( | ||
| len(parts) == 3 |
There was a problem hiding this comment.
When tracecat.json sets resources.workflows to a nested path such as sync/workflows/, this helper splits sync/workflows/foo/definition.yml into four parts and then compares only parts[0] to the full root string, so both read_files() and parse_files() skip every workflow under that valid manifest root. A pull/status for such a repo will see an empty remote spec and can untrack previously synced mappings as if the files were deleted; compare the path against the root's path components instead of assuming the root is one segment.
Useful? React with 👍 / 👎.
1 participant
What
M1 ("Close the loop") for Revamp Git Sync: makes the edit → stage → export → merge → pull cycle complete repeatedly without wedging.
last_synced_spec_hash(repo spec space, for remote drift) andlast_projected_spec_hash(local projection space, for pending changes), recorded from a post-reconcile re-projection that doubles as theP(W_next) == Spostcondition check. Hand-authored repo files omitting optional keys (e.g.webhook:) no longer leave the workspace permanently dirty.export_workflowstops advancing mapping sync metadata — exports land on a branch, so the resource stays pending until the base advances on pull. Status and pending counts can no longer contradict each other after a publish.-Nslug dedup as mapping creation, so duplicate workflow titles can't silently shadow each other or flip status.workspace_sync_eventwas never written; removed from models, RLS config, and the migration. The CRUD journal ships post-v1.include_headers(feat(webhooks): add include_headers toggle to webhook trigger #2837) through the projection so it survives the store round-trip.Database
Edits the (unreleased, this-stack-only)
25f4e2a1c9d8migration in place: reparents onto main's head9b52f7f18a31, folds inrendered_files, addslast_projected_spec_hash, dropsworkspace_sync_event. Single additive revision, single alembic head.Testing
New regression tests in
tests/unit/test_workspace_sync.py:clean, twice.remote_ahead→ pull →clean).webhook:stayscleanafter pull.ruff,ruff format, andbasedpyrightclean; 25 tests pass across the sync, store service, and publish API suites.Summary by cubic
Closes the Git sync loop so edit → export → merge → pull can repeat. Pull now reconciles per resource, blocks only on true conflicts, handles deletes cleanly, and rebaselines resources that already match.
New Features
last_synced_spec_hash(repo spec) andlast_projected_spec_hash(local projection) recorded post-reconcile; optional-key omissions no longer leave dirty state (ENG-1475).pr_base_branchwhen provided; auto-publish branches get a random suffix; preserve webhookinclude_headers.Migration
25f4e2a1c9d8: addlast_projected_spec_hash, foldrendered_files, dropworkspace_sync_event, and remove redundant unique constraints on sync tables; reparent onto main head. Single additive Alembic head.Written for commit ee74c72. Summary will update on new commits.