# 0037. PR mode's file-finding and byte-preserving YAML edit, as `pkg/prmode` Status: Superseded by ADR 0101 · Date: 2026-10-26 · Design section: GitOps and Argo CD (phase 8, "PR mode") Decided: built `pkg/prmode`'s `ClassifySource`/`Locate`/`Patch`/`Diff` -- finding a tracked CR's file via its Application's source and editing one field with go-yaml's `yaml.Node`, never a PR host API -- as its own host-independent, fully unit-testable unit, separate from opening a PR ## Context architecture.md's phase-8 "PR mode" paragraph does three things in one breath: find the file, edit it preserving formatting, and open a PR whose description carries an impact preview. The first two are pure, local, host-independent operations with no external dependency once a checked-out repository exists; the third needs a GitHub/GitLab/Gitea-Forgejo API call, a network connection, and an impact-preview backend another agent is building concurrently. Mixing them would drag the exhaustively-testable half (formatting preservation under every YAML shape a real manifest uses) into the half that cannot be tested without a live host. This unit is only the first two. ## Decision: `pkg/prmode`, not `internal/cli` Both the CLI and the UI need this (architecture.md: "the UI and CLI don't write it"), so it is a `pkg/` package -- the same reasoning `pkg/gitops` and `pkg/upgrade` already follow -- not something built inside `internal/cli` that the UI backend could not import. ## Decision: `yaml.Node` round-tripping, not unmarshal-to-struct-then-marshal The task's own constraint is correct and was verified, not assumed: a plain unmarshal into a Go struct and marshal back loses comments outright, reorders keys to the struct's field order, and normalizes quote style. `go.yaml.in/yaml/v4` (already this project's own YAML dependency -- `internal/cli/bundlefile.go`, `pkg/gitops` -- a fork-successor of `gopkg.in/yaml.v3` with the same `yaml.Node` API) decodes a document into a node tree whose comments, key order, anchors/aliases, quote style and block/flow list style survive a decode-mutate-encode round trip, **provided the encoder's indent width is set to match the document's own** -- verified empirically before writing any package code: `yaml.NewEncoder(...).SetIndent(n)` defaults to a width that does not match every input, and the first version tried (no `SetIndent` call) silently widened a 2-space document to the library's own default. `detectIndent` (`pkg/prmode/patch.go`) measures the document's own first indented line rather than assuming one width. One limitation surfaced by the same verification and is not worked around: a blank line with no comment attached to it, between two sibling keys *inside the document actually being edited*, is dropped by go-yaml's own Node round trip (blank lines are only preserved when attached to a comment). `TestPatch_BlankLineBetweenKeysIsDropped` (`pkg/prmode/patch_test.go`) records this as a known, intentional fact rather than leaving it to be rediscovered as a bug later. It does not affect any document Patch does not touch, nor any comment-attached blank line, nor any blank line in a document it does touch that is not between two keys both re-encoded. ## Decision: only the matched document is ever re-rendered A file can hold more than one `---`-separated document. `splitDocuments` (`pkg/prmode/yamldoc.go`) partitions a file into raw byte segments on a column-0 `---` line -- safe without a full YAML lexer, because block scalar content is always indented past the key that introduces it, so a column-0 `---` can never be anything but a real document-stream marker. (shortcut: this does not hold inside an open flow collection spanning multiple lines, a shape real Kubernetes manifests do not use.) `Patch` re-encodes only the one segment whose `apiVersion`/`kind`/`metadata.name`/`metadata.namespace` match `Target`; every other segment's bytes, and every separator line, are copied into the result unchanged -- not reused by convention, but never decoded at all. This is also what makes "only the right document changes when a file holds more than one CR" true by construction, not by a careful encoder setting that might regress. ## Decision: editing is single-field, mapping-keys-only `FieldUpdate.Path` is a sequence of mapping keys; `setNode` has no list-index support. architecture.md's "edit the right node" never asks for list-indexed paths, and every field PR mode might plausibly edit today (`TalosCluster.spec.*` and its siblings) is nested maps. Adding index support speculatively is left as a documented shortcut in `setNode`'s own comment, not built. ## Decision: Helm/Jsonnet classification reads raw unstructured fields, not `pkg/gitops.ApplicationSource` alone `pkg/gitops.ApplicationSource` (built for the *write* side, Ichor's own skeleton) has no `plugin` field at all, because Ichor's own Applications never use a Config Management Plugin. Decoding an externally-defined, possibly-Jsonnet-via-CMP Application straight into that struct would make a plugin source indistinguishable from a plain path source -- both would show a non-empty `Path` and an empty `Chart`. `ClassifySource` (`pkg/prmode/source.go`) checks for `spec.source.plugin`'s presence directly on the unstructured object before ever decoding into `ApplicationSource`, so a plugin source is caught as `SourcePlugin` (diff-only) rather than misclassified as editable. `spec.sources` (plural, multi-source Applications) is also read directly and treated as unsupported outright: architecture.md's "find the file via the Application's source" describes one source, and guessing which of several holds a given CR risks editing the wrong repository. ## Decision: `Diff` is a two-string unified diff, nothing more For `SourceHelm`/`SourcePlugin`, architecture.md asks for "a diff to apply by hand" -- but there is no single file to diff against for a templated or plugin-rendered CR, only its current and about-to-be-applied YAML renderings, which the caller already has (from the cluster, not from a file this package would have to locate). `Diff` (`pkg/prmode/diff.go`) is therefore a thin wrapper over `github.com/pmezard/go-difflib` (already an indirect dependency; promoted to direct here rather than hand-rolling a diff algorithm) taking two YAML strings and a display name, with no filesystem or source-resolution logic of its own. ## Consequences - Not built here: opening a PR against any host (GitHub, GitLab, Gitea/Forgejo) -- a later unit's interface, deliberately not designed yet, per architecture.md's own phase-8 framing and this unit's own instructions. The UI following a PR through merge/sync/apply, the impact preview text in a PR's description, strict mode, and override are likewise untouched. - `Locate` takes an already-checked-out directory; cloning or fetching a repository from `ApplicationSource.RepoURL`/`TargetRevision` is the host-specific unit's job, not this package's. - `internal/cli/gitops_tracking.go`'s existing `isTracked` (Argo CD via `argocd-cm`, Flux via the `flux-client-side-apply` managed-fields signal) is reused as-is for "is this CR tracked" -- this unit adds no second tracking check. Resolving *which* Application tracks a CR (so its `spec.source` can be read) for the Flux case is not built: Flux has no single Application-shaped object with a `.spec.source` the way Argo CD does (ownership lives in a Kustomization's `.status.inventory`, pointing at a separate `GitRepository`), and architecture.md's own "find the file via the Application's source" wording is Argo CD vocabulary. `ClassifySource` takes an Argo CD Application directly; wiring "CR -> owning Application" (via the tracking annotation/label's own value) and the Flux equivalent are both left to the caller/a later unit.