Merge pull request #305 from fiddlerwoaroof/docs/1xfb-1-merge-only
docs: reconcile diverging history only by merge (improvise-1xfb.1)
This commit is contained in:
@@ -27,7 +27,7 @@ them here would let the copy drift:
|
|||||||
| `context/design-principles.md` | The "why" — functional-first, polymorphism over conditionals, correctness by construction, layering, §7 testing doctrine |
|
| `context/design-principles.md` | The "why" — functional-first, polymorphism over conditionals, correctness by construction, layering, §7 testing doctrine |
|
||||||
| `context/repo-map.md` | The "where", plus **numbered gotchas** — read the ones your diff touches |
|
| `context/repo-map.md` | The "where", plus **numbered gotchas** — read the ones your diff touches |
|
||||||
| `prolog-sessions/kb.pl` | Executable knowledge: `invariant/2`, `doctrine/2`, `forbidden/2`, `decision/3`, and prior `work_done/3` |
|
| `prolog-sessions/kb.pl` | Executable knowledge: `invariant/2`, `doctrine/2`, `forbidden/2`, `decision/3`, and prior `work_done/3` |
|
||||||
| `AGENTS.md` | Working rules: bug-first tests, `Option`/`Result`/`Box` at the edges, merges over rebases |
|
| `AGENTS.md` | Working rules: bug-first tests, `Option`/`Result`/`Box` at the edges, history reconciled only by merge, and nothing that needs a force push |
|
||||||
| `.agents/skills/wave/agent-rules.md`, "GitHub" | The reviewer's rule: what to read before your first comment, how to answer each existing review, and what every review you post must state. Read it before reviewing a PR |
|
| `.agents/skills/wave/agent-rules.md`, "GitHub" | The reviewer's rule: what to read before your first comment, how to answer each existing review, and what every review you post must state. Read it before reviewing a PR |
|
||||||
|
|
||||||
`kb.pl` is the highest-value and least-obvious of these. Grep it before
|
`kb.pl` is the highest-value and least-obvious of these. Grep it before
|
||||||
|
|||||||
@@ -99,7 +99,8 @@ defect is a test that passes.
|
|||||||
run the gates on the merged tree: `AGENTS.md`, "Merging many PRs at once".
|
run the gates on the merged tree: `AGENTS.md`, "Merging many PRs at once".
|
||||||
- A re-review is of the **delta**, at a named SHA. A review of a superseded
|
- A re-review is of the **delta**, at a named SHA. A review of a superseded
|
||||||
commit reads as a pass.
|
commit reads as a pass.
|
||||||
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`. Close
|
- Merge on GitHub with a merge commit, never a squash or rebase merge
|
||||||
|
(agent-rules, rule 8). Then `git fetch gh && git merge --ff-only gh/main`. Close
|
||||||
the bead only once the merge has landed. That is when the work is done:
|
the bead only once the merge has landed. That is when the work is done:
|
||||||
`AGENTS.md`, "Repository policy over the Beads blocks".
|
`AGENTS.md`, "Repository policy over the Beads blocks".
|
||||||
- Anything posted to GitHub goes out under the owner's account: open every
|
- Anything posted to GitHub goes out under the owner's account: open every
|
||||||
|
|||||||
@@ -32,17 +32,57 @@ not in a second copy.
|
|||||||
6. **The Beads "Session Completion" block near the end of `AGENTS.md` does not
|
6. **The Beads "Session Completion" block near the end of `AGENTS.md` does not
|
||||||
set your git policy.** This repository does not opt in to the block's
|
set your git policy.** This repository does not opt in to the block's
|
||||||
"team-maintainer" profile. On a wave branch that profile's pull and bare
|
"team-maintainer" profile. On a wave branch that profile's pull and bare
|
||||||
`git push` would be refused (rule 4), or worse. Rules 1–5 and 7 are your
|
`git push` would be refused (rule 4), or worse. Rules 1–5, 7 and 8 are your
|
||||||
git policy. The block is bd's and bd regenerates it whole, so it is not
|
git policy. The block is bd's and bd regenerates it whole, so it is not
|
||||||
edited by hand: `AGENTS.md`, "Repository policy over the Beads blocks" (improvise-x1uk).
|
edited by hand: `AGENTS.md`, "Repository policy over the Beads blocks" (improvise-x1uk).
|
||||||
7. **Pin `gh/main` to a literal SHA when the task starts.** Run
|
7. **Pin `gh/main` to a literal SHA when the task starts.** Run
|
||||||
`git rev-parse gh/main` once. Use that SHA, not the ref name, for every
|
`git rev-parse gh/main` once. Use that SHA, not the ref name, for every
|
||||||
reset, merge-base and diff. Remote refs are shared by every worktree of the
|
merge, merge-base and diff. Remote refs are shared by every worktree of the
|
||||||
repository, so `gh/main` moves whenever another session fetches. An agent
|
repository, so `gh/main` moves whenever another session fetches. Re-resolve
|
||||||
ran `git reset --soft gh/main` mid-task and staged a reverse diff of #196.
|
|
||||||
It noticed only because it read `git status` (improvise-diho). Re-resolve
|
|
||||||
only on purpose, for example when you mean to merge a newer main. Rule 3's
|
only on purpose, for example when you mean to merge a newer main. Rule 3's
|
||||||
check before a PR is such a time.
|
check before a PR is such a time.
|
||||||
|
8. **Reconcile diverging history only by merge.** To take a newer main,
|
||||||
|
re-pin on purpose first (rule 7): `git fetch gh`, then `git rev-parse
|
||||||
|
gh/main`. Then run `git merge <new SHA>`. Merge commits and plain
|
||||||
|
WIP commits are what the rule wants.
|
||||||
|
|
||||||
|
**The line is the push.** The user: "I don't mind amending on a branch
|
||||||
|
that hasn't been pushed, but don't do anything that requires a force push"
|
||||||
|
(2026-09-25; improvise-79h5).
|
||||||
|
- **Before your first push**, you may amend, reorder or fold your own
|
||||||
|
commits. You may never move them onto another base: no `git rebase` onto
|
||||||
|
main and no `git reset --soft gh/main`. Both incidents below were exactly
|
||||||
|
that, on commits that had not been pushed.
|
||||||
|
- **Once a commit is pushed**, nothing may rewrite it: no rebase, squash
|
||||||
|
(`git reset --soft`, `git merge --squash`, a `rebase -i` fixup), amend or
|
||||||
|
`git reset` that drops it. Anything that would need a force push is
|
||||||
|
forbidden, and so is the force push itself.
|
||||||
|
|
||||||
|
The user's reason: "the merge structure of git history is a causal record"
|
||||||
|
(2026-09-25; improvise-1xfb.1). A merge keeps that record, and a rewrite
|
||||||
|
replaces it with a story. The rewrite is also where the damage comes from.
|
||||||
|
A squash or rebase computes a new commit against a ref another session can
|
||||||
|
move, and when that ref moves, the commit silently reverts whatever landed
|
||||||
|
in between. It has happened twice, and making the operation safer did not
|
||||||
|
stop the second:
|
||||||
|
- **#196:** an agent ran `git reset --soft gh/main` mid-task and staged a
|
||||||
|
reverse diff of #196. Only `git status` caught it (improvise-diho).
|
||||||
|
- **#303:** after #303 merged mid-session, the improvise-cgqt agent squashed
|
||||||
|
with `git reset --soft gh/main`. The staged commit reverted #303, and only
|
||||||
|
rule 3's numstat caught it before the push.
|
||||||
|
|
||||||
|
A cherry-pick copies a change and drops its causal link, so use a merge
|
||||||
|
wherever a merge does the job. That includes a branch that carries another
|
||||||
|
PR's unmerged commits, when that PR is open and will land as a merge
|
||||||
|
commit. Either stack on that PR, or wait for it to merge, re-pin, and merge
|
||||||
|
the new SHA. The foreign commits then leave your diff.
|
||||||
|
|
||||||
|
The one place a merge cannot help is commits that will never reach main as
|
||||||
|
they are. That means an abandoned or rebuilt PR, or commits that exist only
|
||||||
|
on a mirror or a local main (#148). Only then, cut a fresh branch from your
|
||||||
|
pinned SHA and `git cherry-pick -x` your own commits onto it (`-x` records
|
||||||
|
where each came from). Close the old branch rather than force-pushing it:
|
||||||
|
`AGENTS.md`, "Shipping a wave".
|
||||||
|
|
||||||
## Commit early: a worktree is not durable storage
|
## Commit early: a worktree is not durable storage
|
||||||
|
|
||||||
@@ -50,7 +90,9 @@ Commit a WIP as soon as your first test run is green, and again after every
|
|||||||
meaningful step. Push your branch (rule 4) once it is worth keeping. An agent
|
meaningful step. Push your branch (rule 4) once it is worth keeping. An agent
|
||||||
finished a whole ticket with every gate green and lost all of it, uncommitted,
|
finished a whole ticket with every gate green and lost all of it, uncommitted,
|
||||||
when its worktree was deleted in a disk cleanup. A local commit survives a
|
when its worktree was deleted in a disk cleanup. A local commit survives a
|
||||||
deleted worktree. Tidying history before the PR is fine; losing it is not.
|
deleted worktree. You may tidy your WIPs before your first push. After it,
|
||||||
|
they are history and stay as they are, because rewriting a pushed commit would
|
||||||
|
need a force push (rule 8).
|
||||||
|
|
||||||
## Gates
|
## Gates
|
||||||
|
|
||||||
@@ -168,6 +210,10 @@ out on purpose:
|
|||||||
it; do not reason about it. Report *where* it failed, by case name. An
|
it; do not reason about it. Report *where* it failed, by case name. An
|
||||||
injection that does not compile reads as a clean run, so get a positive
|
injection that does not compile reads as a clean run, so get a positive
|
||||||
signal that the build ran.
|
signal that the build ran.
|
||||||
|
- **Bound any test loop that walks a state machine until a step.** An
|
||||||
|
unbounded `while step != target { advance() }` hangs the suite when the
|
||||||
|
transition it counts on is broken. Loop a fixed number of times and assert
|
||||||
|
the step was reached, so the failure is a named assertion, not a timeout.
|
||||||
- **Check your test against every vacuity shape seen here:**
|
- **Check your test against every vacuity shape seen here:**
|
||||||
- an assertion the default state already satisfies;
|
- an assertion the default state already satisfies;
|
||||||
- `f(x) == f(x)`: calling a pure `Cmd` twice against one immutable context;
|
- `f(x) == f(x)`: calling a pure `Cmd` twice against one immutable context;
|
||||||
|
|||||||
@@ -6,7 +6,11 @@
|
|||||||
from logic
|
from logic
|
||||||
- @context/repo-map.md is your "road map" for the repository. use it to reduce exploration and keep it updated.
|
- @context/repo-map.md is your "road map" for the repository. use it to reduce exploration and keep it updated.
|
||||||
- @context/design-principles.md is also important for keeping the repository consistent.
|
- @context/design-principles.md is also important for keeping the repository consistent.
|
||||||
- prefer merges to rebasing.
|
- reconcile diverging history only by merge: the merge structure of git history is a causal record (the
|
||||||
|
user, 2026-09-25). The line is the push: "I don't mind amending on a branch that hasn't been pushed, but
|
||||||
|
don't do anything that requires a force push" (the user). So never rebase onto main, and never rebase,
|
||||||
|
squash, amend or `reset` away a pushed commit. The rule and its incidents are
|
||||||
|
`.agents/skills/wave/agent-rules.md`, rule 8.
|
||||||
- always start responses with bananaS!
|
- always start responses with bananaS!
|
||||||
- there is a prolog MCP available: load a session from `kb` at the root of the repository at the beginning of
|
- there is a prolog MCP available: load a session from `kb` at the root of the repository at the beginning of
|
||||||
every session, and query it as you work. **Reading only** — the MCP must never write `kb.pl`.
|
every session, and query it as you work. **Reading only** — the MCP must never write `kb.pl`.
|
||||||
@@ -147,8 +151,21 @@ once prescribed exactly that command (see "Repository policy over the Beads bloc
|
|||||||
It cost a review round on PR #145,
|
It cost a review round on PR #145,
|
||||||
where four unmerged commits — including another PR's whole diff — were pulled
|
where four unmerged commits — including another PR's whole diff — were pulled
|
||||||
into a branch based on `gh/main` and had to be rebuilt by cherry-picking.
|
into a branch based on `gh/main` and had to be rebuilt by cherry-picking.
|
||||||
ChatGPT Codex caught it; nothing in CI could, because the result compiles and
|
Today the repair **starts with a merge**. If the foreign commits belong to
|
||||||
passes. The tell is a PR diff naming files the ticket has no reason to touch,
|
another open PR that will land as a merge commit, either stack on that PR (see
|
||||||
|
the stacked-PR rule below) or wait for it to merge. Then re-pin `gh/main` and
|
||||||
|
merge the new SHA, and the foreign commits drop out of your diff. Never revert
|
||||||
|
them in your branch: merging the branch would then land an undo of that PR.
|
||||||
|
|
||||||
|
`git cherry-pick -x` is sanctioned **only** when those commits will never reach
|
||||||
|
`main` as they are: an abandoned or rebuilt PR, or commits that exist only on a
|
||||||
|
mirror or a local `main`, as in #148 below. Only then, cut a fresh branch from
|
||||||
|
the pinned `gh/main` SHA and cherry-pick only your own commits. `-x` keeps the
|
||||||
|
link to each original. Open a new PR, and close the old one without
|
||||||
|
force-pushing it.
|
||||||
|
|
||||||
|
ChatGPT Codex caught the #145 base problem; nothing in CI could, because the
|
||||||
|
result compiles and passes. The tell is a PR diff naming files the ticket has no reason to touch,
|
||||||
so **read your own PR's file list before asking for review** — and diff against `gh/main...<branch>`, never the local equivalent. **The second vector is the worktree base**: a branch cut from the orchestrator's *local* `main` inherits whatever that checkout has merged but not yet pushed, and nothing in the branch says so. That hit PR #148 with another PR's whole diff — including `.beads/`, `kb.pl` and `context/repo-map.md`, the three files agents are forbidden to touch, so the review read as a rule violation rather than a base problem. Note the window
|
so **read your own PR's file list before asking for review** — and diff against `gh/main...<branch>`, never the local equivalent. **The second vector is the worktree base**: a branch cut from the orchestrator's *local* `main` inherits whatever that checkout has merged but not yet pushed, and nothing in the branch says so. That hit PR #148 with another PR's whole diff — including `.beads/`, `kb.pl` and `context/repo-map.md`, the three files agents are forbidden to touch, so the review read as a rule violation rather than a base problem. Note the window
|
||||||
is transient — the two remotes are equal immediately after the orchestrator
|
is transient — the two remotes are equal immediately after the orchestrator
|
||||||
syncs — which makes it worse, not better: the hazard is absent exactly when
|
syncs — which makes it worse, not better: the hazard is absent exactly when
|
||||||
@@ -262,8 +279,13 @@ will never see, CI runs on it, and the commit is nowhere. The tell is the push
|
|||||||
printing `* [new branch]` where you expected an `old..new` range. It happened on
|
printing `* [new branch]` where you expected an `old..new` range. It happened on
|
||||||
#51: the third review round completed four minutes after the merge, and the
|
#51: the third review round completed four minutes after the merge, and the
|
||||||
correction it prompted went to a re-created `wave/b-projection-gaps`. The repair
|
correction it prompted went to a re-created `wave/b-projection-gaps`. The repair
|
||||||
is to branch from `gh/main`, cherry-pick, open a follow-up PR, and delete the
|
is a merge, not a cherry-pick. The PR merged with a merge commit, so the stray
|
||||||
stray re-creation. Check `gh pr view <n> --json state` *before* pushing a fix to
|
branch's base is already an ancestor of `main`. Branch from the pinned
|
||||||
|
`gh/main` SHA, `git merge` the stray branch's SHA, open a follow-up PR, and
|
||||||
|
delete the stray re-creation. The merge brings exactly the fix commits, under
|
||||||
|
their own SHAs, with the link to the PR they answered. A cherry-pick would copy
|
||||||
|
them and drop that link. This recipe works only because PRs here never
|
||||||
|
squash-merge. Check `gh pr view <n> --json state` *before* pushing a fix to
|
||||||
a PR you have not looked at in a few minutes, rather than after.
|
a PR you have not looked at in a few minutes, rather than after.
|
||||||
|
|
||||||
**A stacked PR whose base PR has already merged merges into that stale base
|
**A stacked PR whose base PR has already merged merges into that stale base
|
||||||
@@ -289,9 +311,11 @@ commits if the base did not land.
|
|||||||
|
|
||||||
Merge only once a review of the PR's current head has passed — an adversarial
|
Merge only once a review of the PR's current head has passed — an adversarial
|
||||||
agent's — and every Codex and Copilot finding on the PR has an answer. What a
|
agent's — and every Codex and Copilot finding on the PR has an answer. What a
|
||||||
review must state is agent-rules, "GitHub". Prefer a **merge commit** —
|
review must state is agent-rules, "GitHub". Merge with a **merge commit**. Never
|
||||||
`AGENTS.md` says merges over rebases, and a PR's commits are individually
|
use GitHub's squash or rebase merge: history is reconciled only by merge (the
|
||||||
meaningful.
|
rule at the top of this file). A PR's commits are individually meaningful, and
|
||||||
|
the fix-after-merge recipe above needs the PR's head to be an ancestor of
|
||||||
|
`main`.
|
||||||
|
|
||||||
### Merging many PRs at once
|
### Merging many PRs at once
|
||||||
|
|
||||||
|
|||||||
@@ -136,7 +136,9 @@ environment, where egress leaves through your own network boundary.
|
|||||||
been rejected in review.
|
been rejected in review.
|
||||||
- A merged PR cannot carry follow-up work: restart from the latest default
|
- A merged PR cannot carry follow-up work: restart from the latest default
|
||||||
branch and open a new PR.
|
branch and open a new PR.
|
||||||
- Prefer merges to rebasing (`AGENTS.md`), including when bringing the
|
- Reconcile diverging history only by merge, including when bringing the
|
||||||
default branch into a working branch.
|
default branch into a working branch. Never rebase onto main, and never do
|
||||||
|
anything that needs a force push: unpushed commits may be amended, pushed
|
||||||
|
ones may not (`.agents/skills/wave/agent-rules.md`, rule 8).
|
||||||
- Keep `context/repo-map.md` and `context/design-principles.md` current —
|
- Keep `context/repo-map.md` and `context/design-principles.md` current —
|
||||||
they are the two docs agents are expected to maintain by hand.
|
they are the two docs agents are expected to maintain by hand.
|
||||||
|
|||||||
Reference in New Issue
Block a user