# 0038. PR mode: clone, push and open a PR with `net/http` and the `git` binary, never a host SDK Status: Superseded by ADR 0101 · Date: 2026-10-26 · Design section: GitOps and Argo CD (phase 8, "PR mode") Decided: `pkg/prmode` now clones a repository, pushes a branch via the `git` CLI (never `go-git`), and opens a PR/MR via one `net/http` POST per host (never a GitHub/GitLab/Gitea SDK) -- with the token carried only through `git`'s own `GIT_CONFIG_*` environment-variable mechanism and an HTTP request header, never argv, a URL, or a log line ## Context ADR 0037 built `ClassifySource`/`Locate`/`Patch`/`Diff` and deliberately stopped before "open a PR" -- a host-specific API call this unit now builds, finishing the half of phase 8's "PR mode" paragraph that does not include merging it or the UI following one. ## Decision 1: no host SDK `go-github`, `go-gitlab` and `code.gitea.io/sdk` were not evaluated further than noting that each would be the *only* reason to depend on it: creating a pull/merge request is one `POST` per host, with three small, host-specific JSON body shapes (`pr_hosts.go`). `postJSON` (same file) is the one shared `net/http`/`encoding/json` helper all three call. No SDK was added, and this was not escalated back, since the task was explicit that adding one would be. ## Decision 2: push options were checked and rejected, not skipped Checked against each host's own documentation (2026-10-26), not assumed: - **Gitea** has no push option for pull-request creation at all -- only `repo.private` and `repo.template` (`docs.gitea.com/usage/push-options`). Nothing to use there; the REST API is the only path regardless. - **GitLab** does have `merge_request.create`/`.title`/`.description`/ `.target` push options. But two things make the REST API still the better choice even where this exists: (a) a push-option-created MR's URL comes back only as human-readable text in the push's own remote output (e.g. `remote: View merge request for ... at `), not a structured field -- parsing that reliably is a worse bet than the one JSON response field (`web_url`) the REST call already returns for free; (b) GitLab's own docs do not document newline or formatting behavior for `merge_request.description` passed as a single `-o` argument value, and this unit's description needs a multi-line diff, which the REST API's JSON body transport has no such ambiguity about. So this unit uses the REST API uniformly across all three hosts, not a mix -- investigated, not guessed, and the reasoning above is why "two hosts' worth of code disappears" did not happen: only one host even has the option, and using it there would trade a reliable structured response for an unreliable text-parsed one. ## Decision 3: the `git` binary via `os/exec`, not `go-git` This project already depends on `git` being present (`flake.nix`, CI); adding `go-git` would be a second, large way to do the same clone/commit/push this unit needs and nothing else. The decisive factor, as asked: **how the token is passed in**. `os/exec` lets the token travel only through the child process's environment (invisible to `ps`, unlike argv) via `git`'s own `GIT_CONFIG_COUNT`/`GIT_CONFIG_KEY_n`/`GIT_CONFIG_VALUE_n` mechanism (`git-config(1)`, "ENVIRONMENT") setting `http.extraHeader` to an HTTP Basic header -- no config file, no `-c` argument, no URL-embedded credential. `go-git`'s own `http.BasicAuth` struct would also keep the token out of argv (it is in-process, not a subprocess argument at all) and was not rejected for leaking it; it was rejected for being a second git implementation this project would then depend on for no behavior the `git` binary does not already provide via a well-documented, scriptable mechanism. ## Decision 4: the token's path, and how "it never leaks" is tested `gitAuthEnv` (`git.go`) is the one place the token is read: it returns environment-variable key/value pairs, never touching `exec.Cmd.Args`. The remote URL passed to `git clone`/`git push` carries no credential at all -- auth is 100% header-based. `GIT_TRACE`/`GIT_TRACE_PACKET`/`GIT_TRACE_CURL`/ `GIT_CURL_VERBOSE` are explicitly cleared (set to the empty string, not just left unset) in every subprocess's environment, because any of them would print header values -- including this one -- into output this package does capture and can return in an error. `openGitHubPR`/`openGitLabPR`/ `openGiteaPR` send the token only as a request header, never in a URL or a JSON body field, so nothing server-echoed into a response or error body can repeat it either. Tested two ways, both without touching a real host (as required): - `TestGitAuthEnv_CarriesTokenOnlyInEnvNotArgv` asserts the token appears in exactly the one expected `GIT_CONFIG_VALUE_0` entry and nowhere else in the returned environment slice. - `TestCloneAndCommitAndPush_TokenNeverLeaks`/`TestClone_ErrorNeverLeaksToken`/ `TestOpenPullRequest_ErrorNeverLeaksToken`/`TestSubmitPR_TargetNotFoundFailsLoudlyAndNeverOpensAPR` force real failures (a bad branch name, a nonexistent remote, a 422 response, a CR absent from the repository) against a local bare repository (`git init --bare`, no network) and a `net/http/httptest` server, and grep every returned error string for the fake token, finding it nowhere. This does not prove any real GitHub/GitLab/Gitea instance accepts this exact Basic-auth-with-arbitrary-username convention -- that would need a live host, which this unit's own tests are not allowed to touch. The username (`x-access-token`) follows GitHub's own documented convention for a token-as-password login and is used uniformly for all three hosts, since HTTP Basic auth with a token as the password is the one git-smart-HTTP mechanism all three document. ## Decision 5: `Diff` moved from `go-difflib` to `git diff --no-index` ADR 0037's `Diff` used `github.com/pmezard/go-difflib`, promoted to a direct dependency with zero non-test callers at the time -- caught by the coordinator's own `go mod why` check before this unit's `SubmitPR` gave it one. Since this unit already depends on the `git` binary for Clone/ CommitAndPush, `Diff` now shells out to `git diff --no-index` (which needs no repository at all -- it compares two plain files) instead, removing the library dependency entirely with no change to `Diff`'s own signature or any caller. ## Decision 6: `SubmitPR` is the one orchestrator; `AuditRecord.actor` is a plain string parameter `SubmitPR` (`submit.go`) is the single entry point stringing Clone -> Locate -> Patch -> CommitAndPush -> Diff -> `OpenPullRequest` together -- the whole "find the file via the Application's source ... open a PR" path, short of ever merging it (no code path here calls a merge/accept endpoint on any host). It only applies to `SourceFile` Applications; `SourceHelm`/ `SourcePlugin` have no single file to locate, so a caller falls back to `Diff` alone for those (architecture.md's "diff to apply by hand"), not `SubmitPR`. "Who asked" (architecture.md: "a PR whose description includes ... who asked") is `SubmitPRRequest.Requester`, a plain string -- this package never reads `AuditRecord` or imports `internal/uiapi`. Today's actual value a caller passes is the fixed `actor` constant `internal/uiapi/machine_service.go` already defines (`"ichor-ui"`): there is no per-caller identity yet (OIDC login is phase 9), so "who asked" is honestly the backend's own identity, not a person, until that lands -- exactly as `actor`'s own existing doc comment already says. `SubmitPRRequest.ImpactPreview` is likewise an opaque string slot, included verbatim in the PR body when non-empty and never computed here: the impact-preview backend does not exist yet (`rg -i impactpreview` finds nothing), and is a different agent's unit. ## Consequences - Not built here (explicitly out of scope): anything in `internal/cli` (this unit stops at `pkg/prmode`'s own package API, per instruction, to avoid colliding with concurrent `internal/cli` work); strict mode; override; the UI following a PR through merge/sync/apply; computing an impact preview. - `pkg/prmode` now depends on the `git` binary being on `$PATH` at runtime (already true for `make`/CI via `flake.nix`), in addition to the Go dependencies ADR 0037 already introduced. - `SubmitPR`'s shallow (`--depth 1 --single-branch`), temp-directory clone is deleted (`os.RemoveAll`) once the function returns, success or failure; nothing it creates persists past one call.