Skip to content

feat(governance): add downstream sync provenance mode - #33

Merged
joeshull merged 8 commits into
internal/mainfrom
sync-provenance
Aug 22, 2026
Merged

feat(governance): add downstream sync provenance mode#33
joeshull merged 8 commits into
internal/mainfrom
sync-provenance

Conversation

@joeshull

Copy link
Copy Markdown

Downstream governance: true

Problem

The internal/main and main rulesets were active but their ref patterns contained literal quotes (refs/heads/"internal/main"), so they matched no refs and enforced nothing. With the patterns corrected, a conflicted main -> internal/main sync pull request became unresolvable by anyone but the sync app:

  • direct push to internal/main — blocked by the pull_request rule
  • GitHub web conflict editor — commits to main, breaking the mirror
  • side branch -> PR — reconcile is a required check and no existing mode passes (upstream-import wants per-PR upstream metadata; downstream-governance restricts paths to the allowlist; downstream-feature forbids .github/workflows/**, which upstream merges always touch)

Change

Adds a fourth exception mode, Downstream sync: true. It is gated on the pull-request head containing fork main verbatim (comparison status ahead or identical) plus one non-author human approval of the current head, so it cannot be used to smuggle unrelated work in. Changed paths are unrestricted because the content is upstream's plus the conflict resolution.

Test plan

  • node --test scripts/check-upstream-provenance.test.mjs — 16/16
  • Full CI governance set — 41/41
  • npm run format:check clean, npm run typecheck clean
  • All changed paths are in DOWNSTREAM_GOVERNANCE_PATHS, so this PR qualifies for its own Downstream governance: true marker

Conflicted main -> internal/main sync pull requests had no resolution
route that could satisfy the required reconcile check.
…ummary

Review found the mode could not pass CI at all: writeEvidence fell through
to the upstream-import renderer and dereferenced mergeParents. It also found
the ahead/identical containment check was satisfied by any branch that merges
main, leaving one approval to authorise unrestricted paths.
@joeshull

Copy link
Copy Markdown
Author

Review found two real defects. Both fixed in 7c989b1.

The mode could not pass CI at all. writeEvidence early-returns for the feature and governance modes, then falls through to the upstream-import renderer, which dereferences evidence.mergeParents.join(). downstream-sync has no such field, so every run would have thrown TypeError and failed reconcile with a message naming neither the mode nor the cause. This PR stayed green only because it uses Downstream governance: true — the sync path was never executed. Reproduced against the real function before fixing, and there is now a writeEvidence test covering all four modes, which is the coverage whose absence let it ship.

The containment check did not constrain content. compare/main...HEAD returning ahead is satisfied by any branch that merges main, so combined with the skipped path validation, one approval could have landed arbitrary changes to the workflows and to the provenance checker itself — strictly more permissive than downstream-feature, on the noisiest diff in the repo. Replaced with the merge-shape assertion already used by reconciledImportEvidence: the head must be a two-parent merge whose second parent is the live refs/heads/main SHA and whose first parent is an ancestor of internal/main. That is exactly the commit shape the documented workflow produces, and anything riding along now fails the gate.

Also addressed: heads/main is resolved explicitly and compared SHA-to-SHA instead of passing a bare ref name; changedFiles() runs for sync mode again, restoring both the audit record and the validateChangedFileCount anti-truncation guard; approval failures name the mode that required the approval; docs/fork-governance.md no longer claims a guarantee the checker does not enforce, and now documents the post-merge merged_by self-certification caveat.

Not addressed, deliberately: downstreamSyncEvidence itself stays untested because it needs gh, consistent with directImportEvidence and reconciledImportEvidence. Nothing asserts that sync mode reaches effectiveApproval — deleting that line would still pass. Worth a follow-up if the glue is ever made injectable.

Tests 41 -> 44, format and lint clean.

GitHub refuses to let a PR author approve their own pull request, so
the self-approval fix in #34 was necessary but not sufficient: a
solo-maintainer PR could still never produce an APPROVED review,
which permanently deadlocked the required reconcile status check.
Provenance metadata is the audit trail now; no mode asserts a review.
# Conflicts:
#	docs/fork-governance.md
#	scripts/check-upstream-provenance.mjs
…nance

# Conflicts:
#	docs/fork-governance.md
#	scripts/check-upstream-provenance.mjs
@joeshull

Copy link
Copy Markdown
Author

Rebased onto #35 (relax-reconcile) in f5493d5. #35 should merge first — this branch now contains its commits.

#35 found something that invalidates part of this PR's design, and I want it on the record rather than buried in a rebase. Sync mode required a human approval. GitHub blocks self-approval at the platform level, and the other four zge-devs members have zero involvement in this repo — no PRs, no issues, no commits. So that approval could never have been produced, and this PR as originally written would have deadlocked conflicted sync pull requests: exactly the failure it exists to remove. The earlier review and I both evaluated whether the approval was strong enough and neither of us asked whether it was obtainable.

Changes in the rebase:

  • Dropped the approval from sync mode; downstreamSyncEvidence no longer calls effectiveApproval, and the summary renderer no longer emits approval lines. No approval machinery is referenced anywhere in the sync path.
  • Deleted the approval-message test, which tested a function fix(governance): drop the human-review requirement entirely #35 removed.
  • Kept the merge-shape gate: head must be a two-parent merge whose second parent is the live refs/heads/main SHA and whose first parent is an ancestor of internal/main.
  • Reconciled the doc with fix(governance): drop the human-review requirement entirely #35's "no mode requires review" section and corrected its mode count to four.

What sync mode now guarantees, stated plainly so nobody has to re-derive it: the merge shape constrains commit topology, not tree content. A merge commit can carry arbitrary content as "conflict resolution" and this gate will pass it. With review gone repo-wide, nothing mechanical stops an unrelated edit riding along in a sync pull request. That is a deliberate choice, not an oversight — the alternative considered was recomputing the merge with git merge-tree and permitting tree differences only at genuinely conflicted paths, which was declined as disproportionate for a fork where governance mode already permits unreviewed edits to this very checker.

Tests 38/38 (the count moved because #35 removed the approval suite), format and lint clean.

# Conflicts:
#	docs/fork-governance.md
#	scripts/check-upstream-provenance.mjs
#	scripts/check-upstream-provenance.test.mjs
@joeshull
joeshull merged commit ab8e546 into internal/main Aug 22, 2026
20 checks passed
@joeshull
joeshull deleted the sync-provenance branch August 22, 2026 17:57
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