Git workflow discipline

Git Workflow Discipline

The git workflow phases are not suggestions. Each phase has a precondition that the previous phase satisfies; skipping or reordering them breaks the release integrity invariant and can silently corrupt the canonical-path sentinel (ADR-051), invalidate PR review authority, or produce unsigned tags.

The CEO and every spawned agent executing multi-commit work MUST follow the sequence encoded in task-chains.yaml:git-workflow-canonical. This SKILL carries the doctrine — the rationale, the hard rules, the naming conventions, and the anti-patterns — for agents executing each step.

---

What This Skill Is (and isn't)

IS: Doctrinal content for agents executing the git-workflow-canonical task-chain. The pipeline step names in task-chains.yaml point here for semantic grounding. Reading this skill is mandatory before any agent authors a branch, commit, PR, or release tag in {{PROJECT_NAME}}.

IS NOT: A hook, a CI check, or a mechanical enforcer. The mechanical invariants live in:

• check_canonical_edit.py — guards canonical-path writes
• check_plan_edit.py — guards plan-frontmatter transitions
• .github/workflows/validate.yml — enforces test + governance gates
• Branch-protection rules (see docs/BRANCH-PROTECTION.md)

This skill explains WHY those mechanisms exist and what the agent must do BETWEEN the mechanical gates.

---

Phase Sequence (Mandatory)

The six phases form a directed acyclic graph with strict preconditions. No phase may begin until its predecessor's verify condition is met.

Phase 1: branch

Precondition: Main branch at HEAD, clean working tree (git status shows nothing staged or modified).

Action:

git checkout main && git pull --ff-only origin main
git checkout -b <branch-name>

Verify: Branch exists locally. git log --oneline -1 matches main HEAD.

Rules:

• Branch from main exclusively. Never branch from a stale branch or a feature branch (avoids merge-train collisions).
• Use the naming conventions in §Branch Naming below.
• If git pull --ff-only fails, STOP. Resolve main divergence before branching. Never force-push to resolve.

---

Phase 2: draft-commits

Precondition: Branch created in Phase 1. Working tree clean at branch point.

Action: Make the minimum necessary changes. Stage specific files by name. Commit with a compliant message (see §Commit Message Conventions).

Verify: git diff main...HEAD shows only the files the plan authorized. Each commit covers exactly one logical change.

Rules:

• Never git add -A or git add . — only add specific files.
• One logical change per commit. "Logical change" = a unit that can be reverted independently without breaking the system.
• Run the test suite and type-checker before the first commit. A failing suite is not a commit blocker — but the commit message MUST note the failure if committing intentionally broken state (e.g., mid-refactor).
• Never amend a commit that has been pushed for shared review (see Hard Rules §5).

---

Phase 3: PR-open

Precondition: At least one commit on the branch. Tests passing (or failure documented). Branch pushed to remote.

Action: Open the PR with a structured description:

## Summary
<1-3 bullet points>

## Test plan
[Bulleted checklist of what was tested and how]

## Risk / blast radius
[L1-L5 classification per code-review-checklist skill]

Verify: PR exists in remote. Title is ≤70 chars. Description contains Summary + Test plan + Risk sections. At least one reviewer requested.

Rules:

• PR title is the squash-commit message. Write it as if it IS the commit.
• Scope is singular: one logical feature, one bug fix, one refactor. Kitchen- sink PRs trigger mandatory scope split before review proceeds.
• Reference the plan number in the PR body: Plan: PLAN-NNN.
• Never open a draft PR and immediately request review — draft means not ready. Flip to "ready for review" only when Phase 2 verify is complete.

---

Phase 4: review-gate

Precondition: PR open (Phase 3). Reviewer assigned.

Action: The Staff Code Reviewer archetype (see core/code-review-checklist) runs a full review pass. CEO waits for a VETO-clear verdict before proceeding.

Verify: Review verdict is one of: APPROVED / APPROVED WITH CONDITIONS / BLOCKED. BLOCKED verdict → return to Phase 2 (re-commit to fix). APPROVED WITH CONDITIONS → conditions documented and controls named before merge.

Rules:

• Review is not optional. "This is a small change" is not a reason to skip.
• The VETO floor (ADR-052) applies: changes touching auth, financial math, canonical-path writes, or VETO-protected domains require the Opus 4.8 code-reviewer archetype.
• Reviewer cannot be the sole author of all changed lines. Self-review is permitted only for L1 changes (< 50 LoC, single file, no security path). For all other changes, a second archetype must review.
• A "LGTM" without findings is a review-quality failure (see core/code-review-checklist §Anti-Patterns in Reviews).

---

Phase 5: merge

Precondition: Phase 4 verdict is APPROVED or APPROVED WITH CONDITIONS (with conditions satisfied). CI is GREEN on the test-merge commit (refs/pull/N/merge), the merge-queue result, or the equivalent test-merge artifact — NOT only on the PR branch HEAD. Per core/evidence-based-qa Hard Rule #8, PR-branch-only CI does not prove the merged result is green; the target branch may have diverged.

Action:

• Squash merge (default): collapses all draft commits into one clean commit on main. The squash-commit message = the PR title + body summary.
• Merge commit (repo policy override): used when preserving commit history matters (e.g., a long-running feature branch with meaningful individual commits).
• Rebase merge: used when the branch has 2-3 clean, atomic commits that belong in main history individually.

Verify: git log --oneline origin/main shows the merged commit. Working tree on main is clean. CI on main is GREEN post-merge.

Rules:

• Never merge a PR with a BLOCKED review verdict.
• Never merge if CI is RED. Fix CI before merging, even for "obviously correct" changes.
• Delete the branch after merge (remote + local). Stale branches are governance noise.
• If a merge conflict arises at merge time, return to branch, rebase from main (not the reverse), resolve conflicts, re-push, re-run review-gate if the conflict touched reviewed lines.

---

Phase 6: tag

Precondition: Merge complete (Phase 5). CHANGELOG, VERSION file, and NPM package.json version are ALREADY in the merged commit on main — release-meta sync happens during draft-commits (Phase 2) on the release branch, NOT after merge. Tagging is a verification + signing step, not a metadata-bump step. Never commit directly to main. If release metadata is missing from the merged commit, return to draft-commits in a new release branch + PR; do not amend-commit on main and do not "quick fix" via git commit -am.

Action:

# Verify release metadata is present in the merged commit before tagging:
git log -1 --name-only origin/main | grep -E "CHANGELOG|VERSION|package.json"
# Tag the verified commit (signed):
git tag -s v<MAJOR>.<MINOR>.<PATCH> -m "v<MAJOR>.<MINOR>.<PATCH>"
git push origin v<MAJOR>.<MINOR>.<PATCH>

Verify: git describe --tags --abbrev=0 returns the new tag. GitHub release workflow (.github/workflows/release.yml) triggers on the tag push and passes all 14 gates.

Rules:

• Tags follow semver: MAJOR.MINOR.PATCH. No v1.2 short tags. No v1.2.3-hotfix. Use -rc.N for release candidates.
• Tags on canonical releases MUST be GPG-signed (-s). A tag without a signature on a release is a governance failure.
• Never re-tag. If the tag is wrong, the release is wrong. Cut a patch release. Moving a tag breaks reproducibility for every consumer.
• CHANGELOG entry must precede the tag. The format is ## [VERSION] — YYYY-MM-DD — <one-line summary>.

---

Hard Rules

1. Never force-push to main. git push --force origin main is a history-rewrite that invalidates all open PRs, corrupts audit trails, and loses commits for collaborators. There is no valid use case.

2. Never force-push to a shared branch. A branch is "shared" as soon as it exists on the remote AND another agent or human has pulled it or opened a PR from it. Force-pushing to a shared branch causes divergence for every downstream pull.

3. Never amend after pushing for shared review. Once a commit is pushed and a PR is open, amending that commit rewrites history on the remote branch. Instead: make a new commit with the fix. The PR review history stays coherent.

4. Never bundle unrelated changes in one commit. One commit = one logical change. "While I'm here" additions are scope creep disguised as efficiency. They make blame harder, rollback more dangerous, and review harder. The core/minimal-change-discipline skill governs this invariant.

5. Never skip the review gate. The review gate exists because "small" and "safe" are not synonyms. The most catastrophic production incidents originate in one-line changes. If repo policy provides a documented lightweight or docs-only review path (e.g., one-reviewer-with-CODEOWNERS for docs/** paths), USE that documented path — do not invent ad-hoc skips. Every change goes through SOME review path; "I documented why I skipped" is not a substitute for review and is not permitted.

6. Never commit to main directly. The only commits that land on main are merge commits (or squash-merge commits from PRs). Direct commits to main bypass the review gate.

7. Always add specific files, never git add -A or git add .. Wildcard staging can accidentally include .env, credentials, binary blobs, or generated files that should not be versioned. Stage by explicit path.

8. Canonical-guarded paths require a GPG-signed sentinel before merge. check_canonical_edit.py enforces this mechanically. A merge attempt that bypasses the sentinel hook is a governance failure, not a shortcut.

9. A failing CI gate is NEVER merged with "we'll fix in follow-up." Fix the CI gate before merging. If the failure is a known infrastructure flake (not code-related), document the specific flake with a tracking issue and Owner explicit approval before merging.

10. Release tags are GPG-signed and never moved. Signed tags are an integrity guarantee. Moving a tag destroys that guarantee. A wrong tag → cut a patch release.

11. Every merge to main has a tracking ticket or plan reference. The PR description cites Plan: PLAN-NNN or an issue number. Anonymous merges with no plan reference are governance gaps.

12. Rebase from main before requesting review if branch diverged. A PR from a stale branch may have merge conflicts that the reviewer cannot evaluate. Rebase first; request review second.

---

Commit Message Conventions

The framework uses Conventional Commits (https://www.conventionalcommits.org/).

Format

<type>(<scope>): <imperative-mood-summary-≤72-chars>

[optional body — explain WHY, not WHAT; WHAT is in the diff]

[optional footers]

Types

Type	When
feat	New feature visible to users or framework consumers
fix	Bug fix — corrects incorrect behavior
chore	Maintenance — version bumps, dependency updates, tooling
docs	Documentation-only changes (no code, no config)
test	Test additions or fixes with no production code change
refactor	Code restructuring with no behavior change
perf	Performance improvement (measurable, not speculative)
ci	CI/CD config changes
style	Formatting-only (whitespace, linting) — not style opinions
revert	Reverts a previous commit (reference the reverted SHA)

Breaking changes

feat(api)!: change response schema for /orders endpoint

BREAKING CHANGE: `total_price` renamed to `total_amount_cents`. Clients
consuming `/orders` must update field references. Migration guide in INSTALL.md §3.

The ! after the scope signals a breaking change. The BREAKING CHANGE: footer is mandatory for MAJOR version bumps.

Body note pattern

When a commit deviates from a plan or fixes a pre-existing bug discovered in the course of authoring, document it in the body:

fix(security-and-auth): remove phantom --skill/--benchmark flags from CLI invocation

Pre-existing canonical bug discovered during Wave 1a augmentation. The CLI
invocation in the SKILL.md referenced flags that do not exist in the
run-skill-benchmark.py interface. Removed to match actual interface.

NOTE: pre-existing bug fix-forward; deviation from plan scope documented
per Wave 1a pattern (see project_session_91_wave_1a_done.md §exceptions).

Co-Authored-By footer

Every commit authored by a spawned sub-agent must include the footer:

Co-Authored-By: Claude <model-name> <[email protected]>

For example:

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Use a HEREDOC when setting the message in bash to preserve newlines:

git commit -m "$(cat <<'EOF'
feat(plan-074): add git-workflow-discipline SKILL.md

Wave 1b COMPOSITE candidate. Task-chain entry paired at
task-chains.yaml:git-workflow-canonical.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
EOF
)"

---

Branch Naming

<type>/<slug>          (for feature/fix/chore/docs/hotfix branches)
plan-NNN-<slug>        (explicit exception for plan-driven branches)

Prefix	Form	When
feat/	feat/<slug>	New feature or SKILL.md authoring
fix/	fix/<slug>	Bug fix
chore/	chore/<slug>	Maintenance, version bumps, dependency updates
docs/	docs/<slug>	Documentation-only
plan-NNN-<slug>	plan-NNN-<slug>	Work tied to a specific plan — explicit exception to the <type>/<slug> rule because the plan number IS the type signal
hotfix/	hotfix/<slug>	Emergency production fix — goes directly to review-gate

Rules

• Lowercase, hyphen-separated. No underscores, no camelCase.
• Max 50 characters including the prefix.
• Plan-driven branches use the plan-NNN-<slug> form (no slash) — the plan number itself encodes the type signal, so the <type>/<slug> form does not apply. Example: plan-074-wave-1b-git-workflow, not plan/074-wave-1b-git-workflow.
• Never name a branch main, master, HEAD, or any ref name.
• Never reuse a deleted branch name. If feat/order-history was deleted after merge, the next branch touching order history is feat/order-history-v2.

Examples

feat/order-history-pagination
fix/session-token-expiry
chore/bump-anthropic-sdk-0.52.0
plan-074-wave-1b-skill-authoring
hotfix/xss-in-report-renderer
docs/adr-105-accepted

---

WRONG / CORRECT Examples

1. Amending a pushed commit

# WRONG — rewrites history on a shared branch
git commit --amend -m "feat: fix the thing (for real this time)"
git push --force origin feat/my-feature

# CORRECT — new commit, PR history stays coherent
git add src/the-file.ts
git commit -m "fix: correct edge case in the thing (missed in previous commit)"
git push origin feat/my-feature

2. Force-pushing to main

# WRONG — rewrites main, invalidates all open PRs
git rebase -i HEAD~3
git push --force origin main

# CORRECT — if history cleanup is needed, do it on the feature branch
# BEFORE the PR is merged, then squash-merge
git checkout feat/my-feature
git rebase -i HEAD~3   # interactive rebase on feature branch only
git push --force-with-lease origin feat/my-feature  # safe: only YOU have this branch
# then open PR and squash-merge

3. Kitchen-sink PR

# WRONG — PR title: "Misc improvements"
# PR contains:
#   - Adds pagination to order list
#   - Fixes typo in README
#   - Refactors auth middleware
#   - Bumps lodash from 4.17.20 to 4.17.21
#   - Adds dark mode support

# CORRECT — four separate PRs:
#   PR 1: feat(orders): add pagination to order list  (Plan: PLAN-045)
#   PR 2: docs: fix README typo in install section
#   PR 3: refactor(auth): extract session-token validation to dedicated middleware
#   PR 4: chore: bump lodash to 4.17.21 (CVE-2021-23337)

4. Commit-and-pray (no test plan)

# WRONG — PR description:
> "Fixes the thing. Should work now."

# CORRECT — PR description:
> ## Summary
> - Fix off-by-one in order pagination cursor that caused page 2 to skip the
>   last item of page 1 when pageSize was not a multiple of 10.
>
> ## Test plan
> - [x] `test_order_pagination_cursor_boundary` now passes (was failing)
> - [x] Full test suite: `pytest .claude/hooks/tests/ -q` — 3261 passed / 0 failed
> - [x] Manually verified pages 1-3 in staging with pageSize=7, 10, 13
>
> ## Risk / blast radius
> L2 — Adjacent. Touches only `src/orders/paginator.ts` and its test file.
> Rollback: revert the squash-commit. No migration needed.

5. Tag without signature

# WRONG — lightweight tag, no signature, no GPG verification
git tag v1.14.0
git push origin v1.14.0

# CORRECT — signed tag with message
git tag -s v1.14.0 -m "v1.14.0"
git push origin v1.14.0
# Verify: git tag -v v1.14.0

---

Anti-Patterns

1. Commit-and-pray: committing without running tests, relying on CI to catch failures. CI is a safety net, not the primary quality gate. The agent is responsible for test passage before the first push.

2. Reviewers-as-rubberstamp: requesting review as a formality, merging immediately after the reviewer approves without reading findings. Every finding, even NIT, must be acknowledged or disputed before merge.

3. VETO-floor bypass: routing a security or canonical-path change to a Sonnet reviewer to save cost. ADR-052 mandates Opus 4.8 for all VETO-adjacent reviews. Cost is not a valid override.

4. Format-logic bundling: mixing whitespace/formatting changes with logic changes in the same commit or PR. Reviewers cannot distinguish intentional logic changes from formatting noise. Format-only commits are chore/style type; logic commits are feat/fix/refactor.

5. "Cleaning up while here": adding unrelated improvements to a branch opened for a specific fix. Every addition expands blast radius, adds review burden, and risks the primary fix. The minimum-change-discipline skill (see §Related Skills) governs this.

6. Stale branch reuse: re-opening a merged branch for new work instead of creating a fresh branch from main. The merged branch may have diverged from main; working from it introduces drift.

7. Moving tags: re-tagging a commit after a release to "fix" a mistake. The right action is a patch release. Moving tags breaks git's object- identity guarantee.

8. CHANGELOG-after-tag: writing the CHANGELOG entry or bumping VERSION after the tag is pushed. The release workflow validates CHANGELOG content at tag push time. Order: CHANGELOG → VERSION bump → npm sync → commit → tag → push tag.

9. Merging with red CI: "it's just a flake" is never a sufficient justification for merging with a failing CI gate. Flakes are tracked issues with compensating controls; they are not blanket permission to ignore CI output.

10. Branch-name collision reuse: naming a new branch identically to a recently deleted branch. Git may resurrect refs. Use version suffixes.

---

Acceptance Criteria

Before any multi-commit plan execution is marked "done," verify:

• All work occurred on a named branch (never directly on main)
• Each commit message follows Conventional Commits format
• Each commit covers exactly one logical change
• PR opened with Summary + Test plan + Risk sections populated
• Review gate completed (verdict documented in PR or plan directory)
• CI GREEN on the test-merge commit (refs/pull/N/merge), merge-queue result, OR equivalent test-merge artifact before merge — PR-branch-only CI is insufficient (per core/evidence-based-qa Hard Rule #8; the target may have diverged)
• Squash (or merge) commit on main references the plan number
• Feature branch deleted post-merge (local + remote)
• CHANGELOG updated before the release tag (if this plan produces a release)
• Release tag is GPG-signed (if applicable)
• Each commit's staged files match the plan-authorized scope (process requirement — git log --stat shows COMMITTED diffs but does NOT record which staging command was used; auditability comes from reviewer enforcing scope on the diff itself, not from history)
• No force-push to main (verify with git reflog origin/main)

---

Related Skills

• core/code-review-checklist — Review-gate doctrine (Phase 4 preconditions and VETO floor). The Staff Code Reviewer archetype loads this skill for the review-gate phase.
• core/minimal-change-discipline — Governs what goes into each commit (Phase 2). Prevents scope creep and "while I'm here" additions.
• core/devops-ci-cd — CI/CD pipeline configuration that the merge gate (Phase 5) depends on. Read before authoring new GitHub Actions jobs that must pass before merge.
• core/pre-plan-brainstorm — Spec artifact that the review-gate (Phase 4 Pass 1) validates against. Authors the spec.md that the reviewer reads.
• ADR-051 — Canonical-path sentinel discipline; gates canonical-guarded file writes that occur during Phase 2 (draft-commits).
• ADR-064 — Release-gate alignment and model routing; governs which archetype the review-gate (Phase 4) must dispatch.
• docs/BRANCH-PROTECTION.md — GitHub branch protection setup that enforces the merge gate (Phase 5) at the remote level.