Skip to content

Add optional push-before-diff check for Git branches in arc diff#289

Draft
nico97118 wants to merge 1 commit intophacility:masterfrom
nico97118:feature/push-before-diff
Draft

Add optional push-before-diff check for Git branches in arc diff#289
nico97118 wants to merge 1 commit intophacility:masterfrom
nico97118:feature/push-before-diff

Conversation

@nico97118
Copy link

TL;DR

This PR introduces an optional push-before-diff check for Git repositories in Arcanist. It ensures that the current branch is pushed to the expected remote before creating a Differential. Users can configure the behavior via CLI arguments or .arcconfig with flexible modes (error or warn). The feature is fully opt-in, does not push automatically, and provides clear messages with actionable hints. Default behavior is unchanged, and non-Git workflows remain unaffected. This improves safety and prevents creating diffs from local-only commits.

Push-before-diff

Add optional push-before-diff check to ensure Git branch is pushed before creating a Differential diff

1. Background / Motivation

Currently, arc diff will generate a Differential diff from the local working copy without verifying whether the current Git branch has been fully pushed to the remote. In multi-developer workflows, this can lead to diffs being created from commits that are not yet visible to reviewers, causing confusion and potential review of incomplete code.

This becomes problematic in some workflows, e.g., when running CI or automated jobs on branches sent for review.
Also, in my experience with Dev and QA teams, it can be frustrating when fixes reach QA weeks after a developer closes a review, only to find that the commits were never pushed to the repository. While this can be worked around by manually applying the diff, it wastes time and reduces commit traceability.

This PR introduces an optional, opt-in check that enforces that the local branch is pushed to the remote before creating a diff. The feature respects the existing Arcanist philosophy:

  • Provides configurable behavior (warn or error) if the branch is not synced.
  • Works only for Git repositories, with clear messages and HINTs for the user.
  • Can be overridden at the CLI using --no-push-before-diff or configured via .arcconfig.

2. Feature Details

CLI Options Added:

  • --push-before-diff — require the branch to be pushed before diff.
  • --push-before-diff-remote (string) — specify remote to check (default 'origin').
  • --push-before-diff-mode (string) — choose behavior on failure (warn or error).
  • --no-push-before-diff — overrides all checks, bypassing them entirely.

Validation Steps:

Check if an upstream branch is configured for the current branch on the expected remote.

Ensure the upstream remote matches the configured remote.

Verify that the upstream branch exists on the remote.

Ensure all local commits are present on the upstream branch.

Failure Handling:

warn — prints a clear warning and prompts the user whether to continue.

error — throws an exception, with a helpful message including a HINT: "You can bypass this check with '--no-push-before-diff'."

Design Principles Preserved:

Single Responsibility: Arcanist only checks branch state

Backward Compatible: Default behavior remains unchanged. Users must explicitly opt-in.

Consistent UX: Messages are multi-line, with HINTs for corrective action.

3. Use Cases / Benefits

  • Prevent incomplete diffs from being submitted: helps avoid submitting local-only commits to reviewers.
  • Safe opt-in enforcement: teams with strict Git workflows (feature branches, CI pipelines) can enforce pushed diffs without side effects.
  • Clear guidance for users: messages include commands to fix the issue or bypass it safely.

4. Implementation Notes

  • Added validateBranchPushed() to ArcanistDiffWorkflow.
  • Added handleRequirePushedFailure() to centralize handling of warnings/errors.
  • Messages are formatted with multi-line HINTs for readability.
  • Fully configurable via CLI arguments or .arcconfig with defaults (remote=origin, mode=error).
  • Existing functionality for non-Git repositories is unaffected.

5. Why This PR is Low-Risk

Optional feature; does not change default arc diff behavior.
Only affects Git repositories.
No side-effects
Fully testable via existing arc diff workflows.
Clear separation of responsibilities: validation, messaging, failure handling.

6. Suggested .arcconfig Example

{
  "arc.diff.push-before-diff.enabled": true,
  "arc.diff.push-before-diff.remote": "origin",
  "arc.diff.push-before-diff.mode": "warn"
}

This allows teams to enforce the check across the organization while retaining the ability for individual overrides.

Conclusion

This PR improves safety, transparency, and developer workflow consistency for some use-cases without introducing automatic changes or breaking existing behavior. It is fully optional, respects Arcanist’s design principles, and provides clear guidance to the user.

Given that Arcanist is in maintenance mode, this PR is low-risk, backward-compatible, and addresses a real pain point in Git-based workflows.

Introduce an optional, fully opt-in check to ensure that a Git branch
is pushed to the expected remote before creating a Differential diff.

This improves safety in multi-developer workflows by preventing
diffs from being submitted with local-only commits.
Users can configure behavior via CLI arguments :
    --push-before-diff
    --no-push-before-diff
    --push-before-diff-remote (default : origin)
    --push-before-diff-mode (default : error)

or via .arcconfig:
 - arc.diff.push-before-diff.enabled : false
 - arc.diff.push-before-diff.mode    : error
 - arc.diff.push-before-diff.remote  : origin

Modes include 'error' (fail) or 'warn' (prompt).

The implementation leverages Git upstream configuration.
Non-Git workflows are unaffected.
Centralized failure handling ensures clear, multi-line messages with HINTs for users.
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.

1 participant