Skip to main content

Git Workflow

Owner: Joe Etherage Decided: 2026-04-26 (NQU-652 follow-up) Status: Active

Decision: GitHub Flow​

We use GitHub Flow — feature branches off main, PR per change, merge after review:

main ─┬─ feature/something  → PR → merge → main
      ├─ feature/other      → PR → merge → main
      └─ ...

Not GitFlow (no develop branch as integration target). The develop branch that exists at 0428e44 is historical and inactive — see "Why not GitFlow" below.

Branching rules​

  • Branch from origin/main. Never branch from a stale local main; pull first.
  • Branch name format: <owner>/<ticket>-<short-description> for ticket work, docs/<topic> for docs-only, chore/<topic> for maintenance.
    • Examples: cc/nqu-687-pii-scrub-cron, joe/nqu-552-pin-bastion-ami, docs/git-workflow-decision
  • One PR per Linear ticket. Multi-ticket PRs are allowed only when changes are inseparable (e.g., a refactor that touches code paths owned by several tickets) — call this out in the PR description.
  • Stacked PRs are fine when work has hard dependencies — base PR-N on PR-(N-1)'s branch and document the merge order. After PR-(N-1) merges, retarget PR-N to main.

Branch protection on main (NQU-652 Part 1)​

Configured 2026-04-26 via the GitHub API. Settings:

SettingValue
Require PR before mergingyes
Required status checkslint-and-build, test, security-scan (strict — must pass against the latest base SHA)
Required approving reviews1
Dismiss stale reviewsno
Require code owner reviewsno
Force pushesdenied
Branch deletiondenied
Enforce adminsno — Joe retains admin bypass for emergency fixes

The "1 required approval" gate combined with admin bypass means: in normal flow, a reviewer (or eventually a CC/CD agent acting as reviewer) approves before merge; in an emergency, Joe can override via the "Merge without review" admin option. Solo merges by Joe are allowed by design — admin bypass is the escape hatch, not the default.

Why not GitFlow​

The April 2026 staging-to-prod ADR (docs/decisions/2026-04-03-staging-to-production-transition.md) and NQU-652 originally proposed a develop branch + GitFlow. That was the right intuition for a multi-developer team with batched releases. We're a single-developer team (Joe + agents-as-collaborators) shipping continuously to a single environment via Amplify on each main merge. In that shape:

  • The "what's ready but not deployed" intermediate state that develop provides has no consumer — every main merge IS the deploy.
  • Two-PR ceremony (feature → develop → main) doubles review overhead without doubling safety.
  • Hotfixes-against-main + merge-back-to-develop is a known tax that GitFlow imposes; we'd pay it for no benefit.

If/when we add a release cadence (weekly batched deploys, multi-environment staging) the answer flips and we adopt develop. Not now.

The legacy develop branch at 0428e44 is left in place for historical reference but is not a merge target.

CC + CD agent workflow​

When CC or CD does autonomous work:

  1. Always work on a feature branch. Never commit directly to main, even when branch protection isn't enforcing it. The protection is the floor, not the ceiling.
  2. Push the branch and open a PR even when no one will review immediately — the PR is the durable artifact for review, not the local commit.
  3. If multiple commits land for the same ticket, keep them on the same branch.
  4. If unrelated tickets are mid-flight, separate branches.
  5. Declare a review tier in the PR description (see "Review process" below). Joe approves based on description + tier-appropriate review artifacts, not by reading the diff.

CC has had one violation of this so far — the 14-commit autonomous batch on 2026-04-25/26 that was committed directly to main and required remediation into per-ticket branches. The branch protection added at the same time prevents recurrence at the API level.

Review process (added 2026-04-28)​

Joe is the sole approver but does not read code line-by-line — that depth exceeds the time he can give review and isn't where his judgement adds value. The review process is risk-tiered with structured PR descriptions Joe CAN evaluate, plus automated checks doing the technical work.

Tier system​

TierChange typeReview pathJoe time
T0 — auto-mergeDocs, tests, formatting, comments, dependency bumpsCI green → merge. No review.0 min
T1 — description-onlyMechanical refactors, library migrations, internal API moves where behavior is invariantAgent writes detailed plain-language PR description. Joe reads description, asks clarifying questions, approves.2–5 min
T2 — agent self-reviewBehavior changes, new features, DB migrations, anything that changes user-visible flowAgent runs an Agent tool on the diff with a "find problems" prompt and posts findings inline as PR comments. Joe reads description + findings + agent's responses to findings.5–10 min
T3 — ultrareview gateAuth, billing, retention, PHI, security boundaries, FedRAMP-relevant codeJoe launches /ultrareview on the branch. Multi-agent consensus posts back. Joe merges if the agents converge; agent iterates if they don't.10–15 min

Tier assignment​

Agents declare the tier in each PR description with a 1-sentence rationale. Joe can override (escalate) — say T2 → T3 — if extra eyes are warranted. Don't downgrade without explicit approval.

PR description template​

Every PR uses this structure:

## What

<one sentence: what user-visible/operational thing changes>

## Why

<one paragraph: business reason or bug context>

## How (high-level)

<3-5 sentences: approach without code-level detail>

## What could break

<honest list of failure modes considered>

## What I tested

<specific test files + manual verification>

## Tier

<T0 / T1 / T2 / T3> — <1-sentence rationale>

## Migration / infra impact

<dev DB applied? terraform apply needed? feature flag?>

Joe approves intent and risk assessment from this; he does not approve implementation correctness. CI + agent self-review + (for T3) ultrareview cover correctness.

Universal gates regardless of tier​

  • Branch protection requires CI (lint + test + security-scan) green
  • Migrations applied to dev DB before merge (agent ensures + documents in PR)
  • e2e suite green for any T2/T3 PR (agent runs locally, attaches result)

Heads-up for CD specifically​

The above applies to all agents — CC, CD, and Cowork. CD's exposure to PR mechanics is lower than CC's (CD's primary work is specs/prompts/research/design review, not direct code), but when CD does drop code into the repo via Linear MCP or otherwise:

  1. Branch protection on main is live — direct pushes are rejected at the API level
  2. Always open a PR with the description template above
  3. Declare a tier (most CD work is T1 — research/spec docs; design-review work that touches code is usually T2)
  4. Cross-agent: CD writes specs that CC implements. CD's spec landing in docs/claude_desktop/for_claude_code/ does NOT need a PR; it goes to the inbox directory directly. But if CD edits a spec on main (e.g. a published doc), that needs a PR.

Why this shape vs alternatives​

  • Joe-as-line-reviewer: rejected. Doesn't match Joe's strengths or available time.
  • No review (yolo merges): rejected. Single-developer + agents needs SOMEONE checking, and CI alone misses semantic regressions.
  • Outsourced AI review (CodeRabbit, Greptile): considered. Would add T2 self-review automation. Defer until pain is felt; current pattern (agent self-review via Agent tool) covers it without external dependencies.
  • Cross-agent review (CC reviews CD, CD reviews CC): considered. Defer; the Agent self-review pattern catches what cross-agent review would catch, without coordinating two agents.

Mapping to the current 14-PR stack (2026-04-26)​

For reference / first-time application:

  • #67 (workflow doc + branch protection) — T0
  • #68 NQU-677 logAudit fix — T2
  • #69 NQU-673 billing.webhook_processed — T2
  • #70 NQU-608 quality pipeline — T2
  • #71 NQU-606 Phase B+C — T2
  • #72 NQU-618 version_hash data — T2
  • #73 NQU-674 ledger prune cron — T2
  • #74 NQU-678 runbook stubs — T0
  • #75 NQU-679 staleness badge UI — T1
  • #76 NQU-676 search test mocks — T0
  • #77 NQU-606 fail-open — T1
  • #78 NQU-428 load-test harness — T1
  • #79 NQU-428 doc fix — T0
  • #80 NQU-609 Phase 1 retention + legal hold — T3
  • #81 NQU-610 Phase B db consumer migration — T2

Reference​

  • NQU-652 (parent): branch protection + workflow + down-migration policy
  • NQU-652 Part 1 (this doc): branch protection + workflow decision
  • NQU-652 Part 2 (deferred): develop branch — explicitly NOT adopted per the rationale above
  • NQU-652 Part 3 (already shipped): paired .down.sql for new migrations