guard: prove every removal with the forge's answer, not local state

hexajon commented

2026-09-29 18:45:29 +00:00

Owner

ADR 0029 now says that a removal is allowed only when the forge's API confirms that every commit it removes or discards is reachable from the forge's default branch. Local state is not proof. The actor that asks for the removal can move a local ref, rewrite origin/<default> with git update-ref, or fetch and then force-push over a pushed branch.

Today each removal checks only local state:

prune runs git branch --merged against the local default branch.
branch -d relies on git's local merged check.
push --force relies on --force-with-lease, which does not stop a deliberate overwrite.
worktree remove relies on git's clean check, and the branch's commits may exist only on this machine.

Scope:

One proof function in the guard package (#653). It asks the forge whether its default branch holds the given commits, or, for a squash or cherry-pick merge, the given content. A failed forge read refuses.
prune, branch delete, force push, and worktree remove call it on every surface.
Each refusal names the next step: finish the branch, or use raw git in the user's own shell (ADR 0029, ADR 0026).

Acceptance:

Against the live forge and a test repo, each of the four refuses when a commit is held only locally, including when origin/<default> was rewritten to contain it. Each succeeds when the forge's default branch holds the commits.
With the forge out of reach, each of the four refuses.

Depends on #653. Decision record: ADR 0029.

ADR 0029 now says that a removal is allowed only when the forge's API confirms that every commit it removes or discards is reachable from the forge's default branch. Local state is not proof. The actor that asks for the removal can move a local ref, rewrite `origin/<default>` with `git update-ref`, or fetch and then force-push over a pushed branch. Today each removal checks only local state: - `prune` runs `git branch --merged` against the local default branch. - `branch -d` relies on git's local merged check. - `push --force` relies on `--force-with-lease`, which does not stop a deliberate overwrite. - `worktree remove` relies on git's clean check, and the branch's commits may exist only on this machine. Scope: - One proof function in the guard package (#653). It asks the forge whether its default branch holds the given commits, or, for a squash or cherry-pick merge, the given content. A failed forge read refuses. - `prune`, branch delete, force push, and worktree remove call it on every surface. - Each refusal names the next step: finish the branch, or use raw git in the user's own shell (ADR 0029, ADR 0026). Acceptance: - Against the live forge and a test repo, each of the four refuses when a commit is held only locally, including when `origin/<default>` was rewritten to contain it. Each succeeds when the forge's default branch holds the commits. - With the forge out of reach, each of the four refuses. Depends on #653. Decision record: ADR 0029.

hexajon added a new dependency

2026-09-29 18:45:31 +00:00

#653 guard: add the guard package with its three effects

hexajon referenced this issue

2026-09-29 18:52:59 +00:00

docs(adr): consolidate the decision records into their end-state form #667

hexajon referenced this issue

2026-09-29 19:06:46 +00:00

epic: one guard enforces the recorded limits on every surface #670

hexajon added a new dependency

2026-09-29 19:06:52 +00:00

#670 epic: one guard enforces the recorded limits on every surface

hexajon added a new dependency

2026-09-29 19:36:05 +00:00

#674 epic: run forge tests against throwaway Forgejo and Gitea servers behind a real proxy

hexajon referenced this issue

2026-10-03 15:41:18 +00:00

bug(finish): a merged worktree stays on disk until a second call removes it #691

hexajon commented

2026-10-03 20:11:34 +00:00

Author

Owner

Call sites for the branch -d bullet, confirmed by reading the code:

MCP madt_branch_delete: internal/mcp/tools_localgit.go:217 calls DeleteLocalBranch. The tool description there says "purely LOCAL (no remote contact)".
CLI branch -d: internal/cmd/git/branch.go:369 and :407 call the same DeleteLocalBranch.
DeleteLocalBranch: internal/service/git/branch_delete.go:161 runs git branch -d <name> --. No forge client, no forge read.

The check is against HEAD, not the local default branch. branchDeleteMergedIntoRef (branch_delete.go:201-214) runs one --is-ancestor <branch> HEAD probe, then a merged-into-upstream fallback. So the delete is verified against whatever is checked out, or an upstream, which need not be any default branch. "Local state" here is looser than the local default branch.

Reassurance text: the "(merged - its commits are on '')" line (internal/cmd/git/branch.go:195, internal/format/git/branch_delete.go:28) names that local ref. When the proof moves to the forge, this text should name the forge's default branch, not the current branch.

Severity for branch delete, lower than prune (#564): git branch -d accepts only a branch merged into HEAD or its upstream, so its commits stay reachable from HEAD after the delete. There is no silent orphaning as in #564. The fix is about the trust model, a local ref is not the forge's answer, so the regression test should assert a refusal when the commits live only on local state, not recovery from content loss.

Call sites for the `branch -d` bullet, confirmed by reading the code: - MCP `madt_branch_delete`: `internal/mcp/tools_localgit.go:217` calls `DeleteLocalBranch`. The tool description there says "purely LOCAL (no remote contact)". - CLI `branch -d`: `internal/cmd/git/branch.go:369` and `:407` call the same `DeleteLocalBranch`. - `DeleteLocalBranch`: `internal/service/git/branch_delete.go:161` runs `git branch -d <name> --`. No forge client, no forge read. The check is against HEAD, not the local default branch. `branchDeleteMergedIntoRef` (`branch_delete.go:201-214`) runs one `--is-ancestor <branch> HEAD` probe, then a merged-into-upstream fallback. So the delete is verified against whatever is checked out, or an upstream, which need not be any default branch. "Local state" here is looser than the local default branch. Reassurance text: the "(merged - its commits are on '<X>')" line (`internal/cmd/git/branch.go:195`, `internal/format/git/branch_delete.go:28`) names that local ref. When the proof moves to the forge, this text should name the forge's default branch, not the current branch. Severity for branch delete, lower than prune (#564): `git branch -d` accepts only a branch merged into HEAD or its upstream, so its commits stay reachable from HEAD after the delete. There is no silent orphaning as in #564. The fix is about the trust model, a local ref is not the forge's answer, so the regression test should assert a refusal when the commits live only on local state, not recovery from content loss.

hexajon commented

2026-10-09 16:30:19 +00:00

Author

Owner

Reconciling with #635: push --force is omitted from both surfaces, so it leaves this proof's caller set.

This proof fits an operation that destroys a pointer to commits the forge still holds under its default branch. Prune, branch delete, and worktree remove are that shape. The ref goes, the commits stay on the default branch, and the forge's answer confirms it.

Force push is a different shape. It removes commits from the remote branch itself. Force is needed only when the new tip does not contain the old remote tip, so every force push drops at least one commit. The forge holds no retained ref that proves those dropped commits survive, and a rebase is indistinguishable server-side from a deliberate drop. There is no proof to compute, so #635 moves it to omit.

So the proof function has three callers, not four: prune, branch delete, and worktree remove. Drop force push from the scope and the acceptance here.

Reconciling with #635: push --force is omitted from both surfaces, so it leaves this proof's caller set. This proof fits an operation that destroys a pointer to commits the forge still holds under its default branch. Prune, branch delete, and worktree remove are that shape. The ref goes, the commits stay on the default branch, and the forge's answer confirms it. Force push is a different shape. It removes commits from the remote branch itself. Force is needed only when the new tip does not contain the old remote tip, so every force push drops at least one commit. The forge holds no retained ref that proves those dropped commits survive, and a rebase is indistinguishable server-side from a deliberate drop. There is no proof to compute, so #635 moves it to omit. So the proof function has three callers, not four: prune, branch delete, and worktree remove. Drop force push from the scope and the acceptance here.

hexajon commented

2026-10-14 19:00:48 +00:00

Author

Owner

madt_worktrees action=remove removes a clean worktree when its branch holds a commit that is not on the forge's default branch. ADR 0029 requires a refusal in this case.

The remove path has no forge read. The MCP handler worktreesRemove (internal/mcp/tools_worktrees.go:388-396) calls RemoveWorktree (internal/service/git/worktree.go:580-618). RemoveWorktree runs git worktree remove -- <path>, and git checks only that the tree is clean.

Steps to reproduce:

Add a worktree on a new branch from origin/main.
Commit one file in the worktree. Do not push the branch.
Call madt_worktrees action=remove path=<worktree>.

Expected result: a refusal that tells me to finish the branch first.
Actual result: Removed worktree at <path>. The commit stays on the local branch only.

`madt_worktrees action=remove` removes a clean worktree when its branch holds a commit that is not on the forge's default branch. ADR 0029 requires a refusal in this case. The remove path has no forge read. The MCP handler `worktreesRemove` (`internal/mcp/tools_worktrees.go:388-396`) calls `RemoveWorktree` (`internal/service/git/worktree.go:580-618`). `RemoveWorktree` runs `git worktree remove -- <path>`, and git checks only that the tree is clean. Steps to reproduce: 1. Add a worktree on a new branch from `origin/main`. 2. Commit one file in the worktree. Do not push the branch. 3. Call `madt_worktrees action=remove path=<worktree>`. Expected result: a refusal that tells me to finish the branch first. Actual result: `Removed worktree at <path>`. The commit stays on the local branch only.

hexajon referenced this issue

2026-10-15 21:37:10 +00:00

guard: prove a squash-merged branch removable via the forge's merged-PR record #777

hexajon referenced this issue from a commit

2026-10-16 01:29:00 +00:00

feat(guard): #666 require forge proof before a removal destroys a ref

hexajon referenced this issue from a commit

2026-10-16 01:29:00 +00:00

fix(guard): #666 give the forge-reachable test worktree its own branch

hexajon referenced this issue from a pull request that will close it,

2026-10-16 01:29:00 +00:00

refuse a removal the forge cannot prove is reachable #788

hexajon referenced this issue

2026-10-17 16:48:29 +00:00

bug(forge-test): the throwaway-forge binary download times out after a base-snapshot reset #796

hexajon commented

2026-10-17 16:49:03 +00:00

Author

Owner

The code for this landed on main, and the unit tests pass. The live forge test that proves this acceptance, TestForgeProvenRemoval, did not finish in the throwaway-forge VM: the run hit the 10-minute timeout while downloading the Forgejo binary on a cold cache. Tracked in #796. Keeping this open until the suite completes green.

hexajon commented

2026-10-18 05:23:10 +00:00

Author

Owner

The v0.20.0 breaking set is complete and green on main. The code landed across the v0.20.0 branches, the full local gate passes, and the forge suite is green on both forges at both pinned versions on current main (31693766). Closing as done.

The v0.20.0 breaking set is complete and green on main. The code landed across the v0.20.0 branches, the full local gate passes, and the forge suite is green on both forges at both pinned versions on current main (31693766). Closing as done.

hexajon removed a dependency

2026-10-18 05:24:43 +00:00

#674 epic: run forge tests against throwaway Forgejo and Gitea servers behind a real proxy

hexajon closed this issue

2026-10-18 05:25:18 +00:00

Rows
Columns

guard: prove every removal with the forge's answer, not local state #666