feat(gitsync): Phase 2 — write path + sticky proposal branch submit (#38)#40
Closed
chubes4 wants to merge 1 commit intofeat/gitsync-phase-1from
Closed
feat(gitsync): Phase 2 — write path + sticky proposal branch submit (#38)#40chubes4 wants to merge 1 commit intofeat/gitsync-phase-1from
chubes4 wants to merge 1 commit intofeat/gitsync-phase-1from
Conversation
) Adds the full write path to GitSync, layering onto Phase 1's read surface: add, commit, push, submit (the blessed PR flow), and policy-update. Every mutating op runs through policy gates, so a freshly-bound binding stays read-only until writes are explicitly opened up per-binding. ## New abilities (all CLI-only) - datamachine/gitsync-add — stage paths, allowlist enforced - datamachine/gitsync-commit — 8–200 char message, refuses empty - datamachine/gitsync-push — direct push, two-key auth required - datamachine/gitsync-submit — sticky proposal branch + PR - datamachine/gitsync-policy-update — modify policy fields ## Policy gates Four flags on each binding control the write surface: - write_enabled — gates add + commit - push_enabled — gates push + submit - safe_direct_push — second key for push to the pinned branch. push_enabled alone isn't enough — intentional friction so direct push requires deliberate intent. submit() bypasses this because it pushes to a feature branch, not the tracked one. - allowed_paths — empty allowlist = nothing stageable. Every staged path must sit under one of the listed roots. Sensitive-file filter (.env, *.key, credentials.json, etc.) always applies on top. updatePolicy validates: - unknown keys refused - safe_direct_push without push_enabled refused - invalid conflict strategies refused - allowed_paths must be an array ## Submit — sticky proposal branch Each binding gets exactly one feature branch on the remote: `gitsync/<slug>`. Submit rewrites it from fresh upstream + user's edits and opens (or updates) a single PR. No per-submit branches accumulating; the branch represents 'the latest proposal from this binding' and updates in place. Orchestration (GitSyncSubmitter): 1. fetch origin --prune 2. stash dirty + untracked 3. reset --hard origin/<pinned> 4. checkout -B gitsync/<slug> 5. stash pop 6. stage allowed paths (explicit --paths or derived from dirty) 7. commit 8. push --force origin gitsync/<slug> (exclusive ownership) 9. open/update PR via GitHubAbilities 10. checkout <pinned> (always, even on failure) stash-pop conflicts leave the stash in place with a logged warning so edits are recoverable via `git stash list`. Phase 2 supports github.com remotes only for submit (PR backend is GitHubAbilities). Non-GitHub remotes error with unsupported_remote; pluggable backends are Phase 3+. ## Auth github.com push URL is rewritten to include the existing GitHubAbilities PAT (same pattern as homeboy and other DMC git- writing code). Non-GitHub remotes rely on the system's credential.helper. SSH is Phase 3+. ## Shared helper extension Support/PathSecurity gains isPathAllowed(relative, roots) — the empty-allowlist-is-empty-whitelist contract is central to how allowed_paths works, so keeping it in the shared helper means Workspace and GitSync can converge on it if needed later. ## Smoke — local bare repo as fake remote tests/smoke-gitsync-write.php — 35-assertion pure-PHP e2e covering: - Policy gate enforcement (write_disabled, push_disabled, direct_push_blocked, no_allowed_paths, unknown_policy_key, policy_conflict for orphan safe_direct_push, path_not_allowed, sensitive_path) - add + commit + direct push against file:// bare repo - submit orchestration with GitHub API mocked via wp_remote_request stub — verifies PAT header, GET→POST on first run, GET→PATCH on idempotent re-run, sticky gitsync/<slug> branch on remote, working tree returned to pinned after success and failure - Empty-submit refusal when nothing is staged under allowed_paths Uses git's url.<path>.insteadOf config scoped to the clone (no global pollution) to route PAT-injected https URLs back to the bare repo. Phase 1 smokes (32/32 + 32/32) stay green — no regressions. ## Binding shape addition GitSyncBinding::DEFAULT_POLICY adds `safe_direct_push => false`. Existing Phase 1 bindings rehydrate with the default via fromArray's array_merge; no migration required. GitSyncBinding::create also accepts file:// remote_url in addition to https://, http://, and git@. Enables local bare-repo testing without loosening production validation meaningfully — git clone refuses non-repo file:// paths, so no attack surface added. Refs: #38
0d328d0 to
70f9ad6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #39. Review #39 first; this PR's diff is only Phase 2 changes.
Summary
Ships Phase 2 of GitSync (DMC#38) — the write path. Layers on top of Phase 1's read surface (bind/unbind/pull/status/list) with five new abilities that let a binding propose changes upstream. Default posture stays conservative: freshly-bound bindings are read-only until writes are explicitly opened up per-binding.
Implements Model B per design discussion:
add/commit/pushprimitives plus a blessedsubmitthat encodes the PR-based flow.New abilities (5)
datamachine/gitsync-addgitsync add <slug> <paths>…allowed_paths+ sensitive-file filter enforced.datamachine/gitsync-commitgitsync commit <slug> --message=…datamachine/gitsync-pushgitsync push <slug> [--force]datamachine/gitsync-submitgitsync submit <slug> --message=…datamachine/gitsync-policy-updategitsync policy <slug> --flag=…All CLI-only (
show_in_rest=false), all gated byPermissionHelper::can_manage().Policy gates
Four flags on each binding now control the write surface:
write_enabledfalseadd+commitpush_enabledfalsepush+submitsafe_direct_pushfalseallowed_paths[]safe_direct_push=truerequirespush_enabled=true— orphan combinations refused atupdatePolicytime.Typical progression:
submit— sticky proposal branchEach binding gets one feature branch on the remote:
gitsync/<slug>. Every submit rewrites it from fresh upstream + user's edits and opens (or updates) a single PR. The branch represents the latest proposal from this binding — never accumulates per-submit branches.Orchestration (
GitSyncSubmitter::submit):git fetch origin --prunegit reset --hard origin/<pinned>git checkout -B gitsync/<slug>git stash pop--paths=list, or derived from dirty set underallowed_paths)git commitgit push --force origin gitsync/<slug>(we own this branch exclusively)git checkout <pinned>— always, even on failureStash-pop conflicts leave the stash in place with a logged warning so edits are recoverable via
git stash list.Phase 2 supports github.com remotes only for submit — non-GitHub remotes error with
unsupported_remote. Pluggable PR backends (GitLab MR, Gitea) are Phase 3+.Why two keys on direct push?
Bindings model a read-synchronized mirror that tracks upstream. Default posture says "changes go upstream via PR review" (
submit).push_enabledalone letssubmitwork while keeping direct push to the tracked branch gated behind a second, deliberate flag. Half-opting-in (push via submit only) stays safe by default; full direct-push requires explicit intent.Auth
GitHubAbilities::getPat()injected into the push URL ashttps://<token>@github.com/.... Same pattern as homeboy and other DMC git-writing code.credential.helper.Shared helper extension
Support/PathSecuritygainsisPathAllowed(relative, roots). Keeps the empty allowlist = nothing allowed contract in the shared helper so Workspace and GitSync can converge on it if useful later.Binding shape addition
GitSyncBinding::DEFAULT_POLICYaddssafe_direct_push => false.fromArray'sarray_merge; no migration required.GitSyncBinding::createnow also acceptsfile://remote URLs alongsidehttps://,http://, andgit@. Enables local bare-repo smoke testing without meaningfully loosening production validation (git clone refuses non-repofile://paths).Testing
tests/smoke-gitsync-write.php— 35 assertions, all green.Uses a local bare repo as the fake remote (zero network, zero credentials). GitHub API calls are mocked via a
wp_remote_requeststub that captures request shape for assertion.Coverage:
write_disabled,push_disabled,direct_push_blocked,no_allowed_paths,unknown_policy_key,policy_conflictfor orphansafe_direct_push,path_not_allowed,sensitive_pathadd+commit+ direct push against the bare remote (verifies commit lands on remote)submitorchestration: PAT injection into push URL,GET → POSTon first run,GET → PATCHon idempotent re-run, stickygitsync/<slug>branch on remote, working tree returned to pinned after both success and failure pathsallowed_pathsUses
git config url.<path>.insteadOfscoped to each clone (no global pollution) to route PAT-injectedhttps://github.com/...URLs back to the bare repo.Regression check: Phase 1 smokes stay green (32/32 + 32/32).
CLI registration confirmed in Studio:
All 10 abilities confirmed registered.
Files
GitSyncSubmitter.php,smoke-gitsync-write.php)GitSync.php,GitSyncBinding.php,GitSyncAbilities.php,GitSyncCommand.php,PathSecurity.php,docs/gitsync.md)What's next
Phase 3 —
GitSyncPullTask extends SystemTask+datamachine_gitsync_tickrecurring schedule (hourly, opt-in PluginSetting), honoring per-bindingauto_pull+pull_interval. Issue #38 stays open until Phase 3 lands.