sixfold-space/madtea

SHA256

Fork 0

fix: route five git pushes through the guarded writer #858

Merged

hexajon merged 4 commits from issue-833-pushes-through-writer into main

2026-10-21 03:08:19 +00:00

hexajon commented

2026-10-21 03:08:17 +00:00

Owner

Summary

Refs #833

What was wrong

The write guard of ADR 0046 runs inside the Writer in internal/git. Five push calls skipped the Writer and used a raw git runner. They were the two initial-finish pushes in internal/service/workflow/finish_initial.go, the branch push in Develop in internal/service/issue/develop.go, the --push path of runTagCreate in internal/cmd/git/tag.go, and the push in Init in internal/service/workflow/init.go. The guard and the refspec checks did not run on them.

Two more problems showed up while I moved them.

The push runners sent git's stdout to the process stdout. git push -u writes "branch 'x' set up to track ..." there. That text landed on the MCP protocol channel, and before the JSON of a CLI --json run. The standard finish push and madt_push on main had the same fault.
The guard allowed the first push of the default branch only when the local origin/<default> ref was absent. A stale tracking ref (the forge repo was deleted and created again empty) refused a real first push. A missing ref (never fetched) allowed a push onto a forge default that already exists.

What changed

internal/git/writer.go: new Writer.PushHead (push -u origin HEAD:<branch>) and Writer.PushHeadWithCredentials (the same push with explicit credentials, for Init, whose repo holds no credentials yet). Both run the refspec check and the guard before any backend.
The five call sites now push through the Writer: runInitialFinish, runInitialFinishFromFeatureBranch, Develop, runTagCreate (through a new tagPushTag seam) and Init.
internal/git/auth.go and internal/git/ssh_transport.go: both push runners (HTTPS and SSH) now use captureStdoutTeeStderr. It discards stdout. It tees stderr to the process stderr on the CLI only. Under the MCP server nothing streams. The comments on RunGitAuthDirWithStderr and the SSH runner now state this.
internal/service/git/writer.go: the guard reads the forge default branch and the empty flag in one call (forgeRepoStateForDir). writerDefaultState decides the rule. When the forge answers, its empty flag decides. The local tracking ref is the fallback only when the forge cannot be read, so offline behaviour stays.
internal/parity/writeguard_git_drift.go: the five entries leave writeGuardGitAllowlist, so a push around the Writer at these sites fails the drift test again. The Writer-method check now also counts the credentials backend.

What the tests cover

internal/git/writer_head_test.go: PushHead and PushHeadWithCredentials run the refspec check and the guard before a backend.
internal/service/git/writer_hermetic_test.go and writer_test.go: a decision table pins writerDefaultState with stated forge answers, including the stale ref, the missing ref and the offline fallback.
internal/service/workflow/finish_initial_writer_test.go: the initial finish pushes through the Writer.
internal/cmd/git/tag_push_writer_test.go and tag_test.go: tag --push goes through the Writer.
internal/parity/writeguard_git_drift_test.go: the drift test with the smaller allowlist.
internal/testforge/first_push_forge_test.go: forge-tagged tests cover the initial finish into a recreated empty forge repo with a stale tracking ref, and the first push of init.

./scripts/gate.sh passes on this commit.

Closes #833

## Summary Refs #833 ## What was wrong The write guard of ADR 0046 runs inside the Writer in `internal/git`. Five push calls skipped the Writer and used a raw git runner. They were the two initial-finish pushes in `internal/service/workflow/finish_initial.go`, the branch push in `Develop` in `internal/service/issue/develop.go`, the `--push` path of `runTagCreate` in `internal/cmd/git/tag.go`, and the push in `Init` in `internal/service/workflow/init.go`. The guard and the refspec checks did not run on them. Two more problems showed up while I moved them. - The push runners sent git's stdout to the process stdout. `git push -u` writes "branch 'x' set up to track ..." there. That text landed on the MCP protocol channel, and before the JSON of a CLI `--json` run. The standard finish push and `madt_push` on main had the same fault. - The guard allowed the first push of the default branch only when the local `origin/<default>` ref was absent. A stale tracking ref (the forge repo was deleted and created again empty) refused a real first push. A missing ref (never fetched) allowed a push onto a forge default that already exists. ## What changed - `internal/git/writer.go`: new `Writer.PushHead` (`push -u origin HEAD:<branch>`) and `Writer.PushHeadWithCredentials` (the same push with explicit credentials, for `Init`, whose repo holds no credentials yet). Both run the refspec check and the guard before any backend. - The five call sites now push through the Writer: `runInitialFinish`, `runInitialFinishFromFeatureBranch`, `Develop`, `runTagCreate` (through a new `tagPushTag` seam) and `Init`. - `internal/git/auth.go` and `internal/git/ssh_transport.go`: both push runners (HTTPS and SSH) now use `captureStdoutTeeStderr`. It discards stdout. It tees stderr to the process stderr on the CLI only. Under the MCP server nothing streams. The comments on `RunGitAuthDirWithStderr` and the SSH runner now state this. - `internal/service/git/writer.go`: the guard reads the forge default branch and the empty flag in one call (`forgeRepoStateForDir`). `writerDefaultState` decides the rule. When the forge answers, its empty flag decides. The local tracking ref is the fallback only when the forge cannot be read, so offline behaviour stays. - `internal/parity/writeguard_git_drift.go`: the five entries leave `writeGuardGitAllowlist`, so a push around the Writer at these sites fails the drift test again. The Writer-method check now also counts the credentials backend. ## What the tests cover - `internal/git/writer_head_test.go`: `PushHead` and `PushHeadWithCredentials` run the refspec check and the guard before a backend. - `internal/service/git/writer_hermetic_test.go` and `writer_test.go`: a decision table pins `writerDefaultState` with stated forge answers, including the stale ref, the missing ref and the offline fallback. - `internal/service/workflow/finish_initial_writer_test.go`: the initial finish pushes through the Writer. - `internal/cmd/git/tag_push_writer_test.go` and `tag_test.go`: tag `--push` goes through the Writer. - `internal/parity/writeguard_git_drift_test.go`: the drift test with the smaller allowlist. - `internal/testforge/first_push_forge_test.go`: forge-tagged tests cover the initial finish into a recreated empty forge repo with a stale tracking ref, and the first push of init. `./scripts/gate.sh` passes on this commit. Closes #833

hexajon added 4 commits

2026-10-21 03:08:17 +00:00

fix: route five git pushes through the guarded writer e81430590e

The initial finish (both forms), issue develop, tag --push, and init pushed
with a raw git runner, so the write guard and the refspec checks did not run
on them. Each now pushes through the Writer. The Writer gets PushHead
(push -u origin HEAD:<branch>) and PushHeadWithCredentials (the same push
with explicit credentials, for init, whose repo holds no credentials yet).
Both run the refspec check and the guard before any backend.

The parity allowlist loses the five entries, so a push around the Writer
at these sites fails the drift test again. The Writer-method check now also
counts the credentials backend.

Closes #833

fix: keep git push output off the process stdout and quiet under MCP eaf7733c7a

The push runners streamed git's stdout to the process stdout. git push -u
writes "branch 'x' set up to track ..." there, which lands on the MCP
protocol channel and before the JSON of a CLI --json run. This also hit the
standard finish push and madt_push on main.

Both push runners (HTTPS and SSH) now capture stdout. stderr is teed to the
process stderr on the CLI only, so git's progress still reaches the terminal;
under the MCP server nothing streams. The MCP mode is the existing
non-interactive flag that the serve path sets.

fix: decide the first push into an empty repo from the forge 37bd03d7f5

The write guard allowed the first push of the default branch only when the
local origin/<default> ref was absent. A stale tracking ref (the forge repo
was deleted and created again empty) then refused a real first push, and a
missing ref (never fetched) allowed a push onto a forge default that exists.

The guard now reads the forge's default branch and empty flag in one call.
When the forge answers, its empty flag decides. The local tracking ref is the
fallback only when the forge cannot be read, which keeps the offline
behaviour. A decision-table test pins the rule with stated forge answers.

Forge-tagged tests cover the initial finish into a recreated empty forge repo
with a stale tracking ref, and the first push of init.

docs: state the real output disposition in the RunGitAuthDirWithStderr comment

drift / govulncheck (pull_request) Has been skipped

Details

drift / surface-diff (pull_request) Successful in 29s

Details

drift / forgejo-smoke (pull_request) Has been skipped

Details

drift / govulncheck (push) Has been skipped

Details

drift / surface-diff (push) Successful in -42s

Details

drift / forgejo-smoke (push) Has been skipped

Details

ci / gate (pull_request) Successful in 42m54s

Details

ci / gate (push) Successful in 19m42s

Details

a8bb29ec47

The comment pointed readers to RunGitAuthDirCaptureStderr to stream stdout,
but that push runner now discards stdout. It tees stderr to the process
stderr on the CLI and only captures stderr under the MCP server.

hexajon merged commit a8bb29ec47 into main

2026-10-21 03:08:19 +00:00

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea!858

No description provided.

Rows
Columns