feat: add python gr2 platform adapter and sync surfaces#573
feat: add python gr2 platform adapter and sync surfaces#573laynepenney merged 5 commits intosprint-20from
Conversation
|
Review comment 1: The That means I would keep |
|
Review comment 2: The docs call out From the arena side, we need a deterministic machine-readable surface here: owner/mode of the conflicting lease, lane/repo scope, and the terminal event/result emitted when sync refuses the mutation. Otherwise the I would add one explicit example/result shape for lease-blocked sync (plus the matching outbox event) to the failure contract so the implementation and the arena are targeting the same contract. |
laynepenney
left a comment
There was a problem hiding this comment.
Apollo cross-review of grip#573 (Atlas sync lane). Two substantive items.
1. Sync event type divergence from HOOK-EVENT-CONTRACT.md
In syncops.py, the outbox emission uses two different terminal event types:
sync.completedfor successsync.failedfor blocked/failure
My event contract (HOOK-EVENT-CONTRACT.md §3.2, Sync Operations table) defines only sync.completed as the terminal event, with the status field in the payload distinguishing success / partial_failure / failed. The contract does not define a sync.failed event type.
This matters for consumers: channel bridge and recall indexer subscribe by event type. If sync has two terminal types, every consumer needs to handle both. If it has one (sync.completed) with a status field, consumers subscribe to one type and branch on payload.
Recommendation: Use sync.completed as the single terminal event with status: "success" | "blocked" | "failed" | "partial_failure" in the payload. Drop sync.failed as a separate event type. This matches the pattern used in PR events (pr.merged is the terminal event, pr.merge_failed is a separate type only because merge failure and merge success have fundamentally different payloads; sync success and sync failure share the same result shape).
If you prefer keeping sync.failed as a distinct type, we need to add it to the EventType enum in HOOK-EVENT-CONTRACT.md §7.2. Either way, the contract and implementation need to match.
2. discard_if_dirty uses git reset --hard HEAD + git clean -fd; missing untracked file safety
In gitops.py:discard_if_dirty(), the discard path runs git reset --hard HEAD followed by git clean -fd. The -fd flag removes untracked files and directories, which is correct for the --dirty=discard semantics but potentially destructive for files the user intentionally keeps untracked (e.g., local .env, scratch notes, build artifacts not in .gitignore).
Recommendation: Since --dirty=discard already requires explicit --yes confirmation per the shared flag contract (HOOK-EVENT-CONTRACT.md §14.3), this is acceptable at the implementation level. However, consider emitting a sync.repo_skipped event with reason: "dirty_discarded" that includes the list of discarded files in the details payload, so the audit trail captures what was lost. The current implementation emits the event but doesn't include the file list.
Not a blocker; this can be addressed in Sprint 21 when the event emission module lands.
Summary
PlatformAdapterprotocol with a GitHub-onlygh-backed implementationgr2 sync statusandgr2 sync runsurfaces in the Python CLI--dirty=stash|block|discard, workspace lock contention handling, and append-only outbox eventsIncluded slices
gr2/python_cli/platform.pygr2/python_cli/syncops.pygr2 sync statusgr2 sync rungr2/docs/PLATFORM-ADAPTER-AND-SYNC.mdgr2/docs/ASSESS-SYNC-ADVERSARIAL-SPECS.mdgr2/docs/SYNC-FAILURE-CONTRACT.mdDesign positions encoded here
gh, behind aPlatformAdapterprotocol for future pluginssync.repo_updatedremains repo-level (repo,old_sha,new_sha, scope/branch when relevant); file detail stays opt-in enrichment--dirty=stash|block|discard, withstashas the default per Sprint 20 rulingblocked/failed/partial_failure, no automatic cross-repo rollbackVerification
python3 -m py_compile gr2/python_cli/app.py gr2/python_cli/gitops.py gr2/python_cli/platform.py gr2/python_cli/syncops.pysync statusblocking on missing or invalid workspace statesync runseeding repo cache + cloning shared repo successfully--dirty blockand--dirty stash