ADR-0021: Multi-adapter forge seam — forge.RunPort, neutral capabilities, transport policy¶
| Status | Proposed |
| Date | 2026-08-10 |
| Deciders | Operator (maintainer LGTM required — core-contract work per GOVERNANCE) |
| Context links | ADR-0005 (forge abstraction, GitLab first / GitHub second), ADR-0011 (core ports), ADR-0015 §2/§4, ADR-0017 §1/§7, ADR-0019, ADR-0020; D-012/D-017/D-019 (E10 lock), D-140 (E10 unlock); docs/planning/forge-dossier-github.md; docs/planning/design-notes/e10-forge-port-lift.md; audit 2026-08-09 ARCH-18/ARCH-19; spec openspec/specs/p5-e10-github-forge/spec.md |
Context¶
E10 (GitHub adapter) is unlocked by D-140. assent has exactly one forge adapter, and the seam a second adapter must plug into is only half-built:
- The composite port does not exist. AUD-S15 lifted
MRInfoandErrNotFoundintointernal/forge(port.go), butcmd/assent'sforgePortis still an anonymous interface literal declared at the call site, andrun.gostill callsgitlab.SyntheticDigestdirectly.port.go's own scope note records both as E10 work. - The conformance suite cannot be reused. All 1,155 lines of
internal/forge/conformancelive in_test.gofiles, which Go cannot import. The suite that defines "behaves like a forge" is therefore unrunnable by a second adapter — the GitHub adapter would be developed against no executable contract. (Correction, 2026-08-10 adversarial review: an earlier draft of this ADR also claimed the extraction would unblockcatalog.yaml'sgithub-deferredrows. That was false — both such rows arelevel: L3, package: test/e2e, so they are gated on live GitHub infrastructure (E10-S18), not on importability. The extraction's real and sufficient justification is the executable contract.) The deeper problem the same review exposed: the cases assert on*fake.Forgeinternals (sha_guard_test.go:49takes*fake.Forge;reconciliation_test.go:220type-asserts to it), so aFactoryreturning aforge.RunPortis only half the contract — the other half is a port-level observation surface defining what a case is allowed to assert. Without it, the cheap way to make cases run on both forges is to weaken assertions to what both can observe, which is how a suite silently stops proving the SHA-guard. - Capability vocabulary is adapter-private (audit bucket A).
docs/planning/forge-dossier-github.md§4 enumerates eleven capability flags the port needs;probeCapabilitiesreads three project fields, andcapabilityGapis computed in GitLab terms. Arming decisions (ADR-0015 §4) hang off that vocabulary, so a second adapter would either restate it or silently arm under a different meaning of "capable". - Transport policy is adapter-private (audit bucket B). Bounded reads and pagination
caps (AUD-S10), idempotent-GET retry/backoff and context deadlines (AUD-S11) were built
into
internal/forge/gitlab. GitHub additionally needs a GraphQL client (thread resolution is GraphQL-only per dossier §4) and two auth shapes (PAT and GitHub App installation token). Left at the adapter, the two forges' availability and fail-closed behaviour diverge with nothing detecting it.
The design note e10-forge-port-lift.md covers (1) only. Items (3) and (4) are the "two
design buckets" the 2026-08-09 audit flagged as under-scoped; item (2) it flagged
separately.
An adversarial review of the first draft of this ADR (2026-08-10) found three further buckets, two of which are P0. They are recorded here because each is an addressing or representation failure — a class the P1-E3-S03 dossier structurally could not surface, since it studied GitHub's API endpoints, not how the port names things:
- Head-content addressing — the port cannot read a GitHub fork PR's head (P0).
cmd/assent/run.go:281,285reads the governed subject's base and head viaFileAtRef(project, path, ref)withinfo.TargetSHA/info.SourceSHA— a ref inside one project (the refs were branch names before REV1-S01 / D-183 pinned them) — andforge.MRInfocarries no source-repository identifier. On GitHub, a fork PR's head branch lives in the fork, not the base repo, so a ref inside the base project does not address it and the read 404s;fileAtRefOrAbsent(run.go:476-482) mapsforge.ErrNotFoundtonil, andchange.OneSidedLifecycle(base, nil)(internal/change/onesided.go:20-21) returnsKindDelete, true. Every fork PR would be evaluated as a whole-file deletion the contributor never made — a spurious BLOCK, or an APPROVE on fabricated change semantics. FreezingFileAtRef's signature at story 2 without deciding this is the single largest latent refactor in the epic. - Error taxonomy at the port.
forge.ErrNotFoundis not a transport code — it is a semantic presence signal consumed byfileAtRefOrAbsentand turned into aFileEvent. GitHub returns 404 for permission-denied resources too, whereas the GitLab adapter separates 401/403 (ErrUnauthorized) from 404 at the status-code level. Absent an explicit status→sentinel mapping per adapter, "absent" and "forbidden" collapse — the absent-means-trusted pattern the 2026-08-09 audit found three times, arriving by a new route. - Identity at the port.
RunPortcarries no identity; the GitLab adapter smugglesbotAuthorthrough its constructor. "Which artifacts are mine?" — the basis of marker filtering and spoof resistance — becomes a port concept once there are two adapters and two auth shapes (a PAT's identity is aUser, an App's is a bot). - Record surface for multi-capability gaps (P0).
schemas/decision/v1alpha1/decision-record.schema.jsondefines$defs.pinswithadditionalProperties: falseand a single stringcapabilityGap, required iffmergeResultDigestisnull(and forbidden otherwise, via anif/then/else). It models exactly one capability — merge-result pinning — which is why it is singular and coupled to that field. An eleven-capability report has nowhere valid to be recorded, and the epic'sgit diff schemas/ == 0goal forbids widening it. Note the ordering irony: this ADR's normative order puts the port before the capability model, but the capability model has no representation in the frozen contract — so the bucket that had to be decided first is the one nobody enumerated.
All eight must be decided before adapter code, because each one changes what the adapter is written against.
Options¶
| Option | Pros | Cons |
|---|---|---|
| A. Adapter-first — write the GitHub adapter against the existing implicit seam, refactor after | Fastest first commit; concrete code reveals the real seam | The seam gets defined by two accidents instead of one contract; no executable conformance to TDD against; cmd/assent would import a second adapter package, entrenching the ARCH-02 leak the port lift just removed |
| B. Port-first, capabilities and transport left adapter-private | Smaller ADR; matches the design note exactly | Reproduces the audit's under-scope verbatim: arming semantics and availability behaviour stay per-adapter, and the fail-closed guarantee becomes per-adapter rather than per-port |
| C. Port-first with a neutral capability model and port-level transport requirements, conformance extracted to an importable package (chosen) | One executable contract both adapters are proven against; capabilityGap and fail-closed arming mean the same thing on both forges; GitHub's GraphQL/App-auth needs are expressed as adapter-internal freedom under port-level requirements |
Largest up-front cost; five stories land before a single GitHub API call; touches core contract, so maintainer LGTM gates it |
| D. Generalize to a plugin/gRPC forge protocol | Third-party forges without recompiling | No named consumer (D-012 reasoning applies unchanged); freezes a wire contract for a seam with two known implementations; out of scope for v1 |
Decision¶
Adopt Option C. Before any GitHub API call, E10 establishes a single forge seam consisting of four committed pieces:
-
forge.RunPort— a named composite interface ininternal/forge, replacingcmd/assent's anonymous port literal:forge.Forge+forge.Snapshotter+forge.Resolver+Describe(project, mr string) (forge.MRInfo, error)+FileAtRef(project, path, ref string) ([]byte, error)+FileAtBase(mr, path string) ([]byte, error)/FileAtHead(mr, path string) ([]byte, error). The two accessors are not redundant and neither replaces the other — item 5 decides which is legal where.FileAtRefsurvives because policy is ref-addressed by contract: ADR-0015 §1 requires the MergePolicy, RulesetBinding, Config and pack to load from the target ref (the pinnedinfo.TargetSHAsince REV1-S01 / D-183), whichcmd/assent/run.go:208,:216,:235and:254do today and must keep doing. That list is exhaustive forrun.goand NOT forcmd/assent: two further ref-addressed decision-input reads live incmd/assent/provider_host.go— the provider host declaration at:82and the resource-owner registry at:275. Verify against the tree before relying on either list. Two of the six deserve specific mention.run.go:235reads.assent/config.yaml, which carries the provider-host declarations, so migrating it would let a fork's head redefine its own fact semantics.provider_host.go:275is the most dangerous read in the repository to move, because that registry decides who may approve: its own comment records that it once preferred the checkout — which under--checkoutis the merge request's own head tree — letting an MR ship a registry naming its author as owner of the resource it is changing. That escalation was found and fixed as D-130; moving this read onto an MR-relative accessor reintroduces it.FileAtBase/FileAtHeadare the governed subject's only legal accessors. An adapter that implementsFileAtBaseby delegating toFileAtRef(project, path, sourceBranch)reintroduces the defect item 5 exists to kill.cmd/assentdepends onforge.RunPortonly — a depguard rule deniescmd/assentimportinginternal/forge/gitlabandinternal/forge/github, replacing the current three-symbol allowlist. The merge-digest scheme is adapter-owned:gitlab.SyntheticDigestcall-sites collapse ontoSnapshot.Heads.MergeResultDigest. -
An importable conformance package. The case bodies move from
internal/forge/conformance/*_test.gointo importable Go (RunSuite(t, Factory)over aforge.RunPortfactory), leaving thin_test.goentry points per adapter.catalog.yamlremains the index and gains the adapter dimension. A case is the same case on both forges or it is not a conformance case. -
A neutral capability model.
forge.Capabilityis a closed, port-owned enum seeded from dossier §4's eleven flags; adapters return aforge.CapabilityReportofsupported | absent | unknownper capability with an adapter-supplied reason.capabilityGapis computed at the port from that report, never by an adapter, andunknownis treated exactly asabsentfor arming (unprobed is not proof). This is the port-level statement of ADR-0015 §4's "refuses to arm when it cannot verify". -
Port-level transport requirements. Bounded response reads, pagination caps, idempotent-GET-only retry with backoff, and context deadlines become requirements of the port with conformance cases, not properties of one client. Auth shape (PAT vs. GitHub App installation token) and protocol (REST vs. GraphQL) stay adapter-internal freedom — the port never names a transport.
-
An explicit addressing model, decided before the port is frozen. The port stops addressing the governed subject by
(project, branch-name)and instead exposes the two sides of the change relative to the merge request itself —FileAtBase(mr, path)/FileAtHead(mr, path)— leaving each adapter to own how it reaches a fork's head (refs/pull/N/headon GitHub, source-project ID on GitLab). A conformance case must prove that a fork MR with an unchanged governed file yields no lifecycle event, on both adapters. Smugglingrefs/pull/N/headintoMRInfo.SourceBranchis explicitly rejected: it corrupts a documented field and leaks into rendering. Scope of the narrowing, stated precisely because item 1 keeps both accessors: it binds the governed subject only —run.go:281,285, the reads whose 404-maps-to-nilfeedschange.OneSidedLifecycleand mints the fabricated whole-file DELETE. The policy loads atrun.go:208,:216,:235,:254— plusprovider_host.go:82and:275— are deliberately stillFileAtRef(project, path, targetRef): they read the protected target ref of the target project, which is exactly the trust boundary ADR-0015 §1 draws, and a fork's head must never be able to reach them. Rewriting those onto an MR-relative accessor would be a trust-boundary regression, not a cleanup. Consequence accepted: this is a larger refactor than the design note anticipated and it collides with the byte-identical-golden requirement; the goldens are re-proved equal on GitLab rather than assumed. -
A per-adapter HTTP-status → port-sentinel mapping, with a conformance case per sentinel.
ErrNotFoundmeans absent, never forbidden: an adapter that cannot distinguish them for a given endpoint must return an error, not absence. A permission failure must never render as a deleted file. -
Identity is a port concept.
RunPortexposes the authenticated identity, and marker filtering matches that identity — not "any bot". Both auth shapes are covered, with a case proving PAT-mode markers are recognised as our own (otherwise assent is blind to its own comments and duplicates them forever). -
The capability report's record surface is decided here, not in a story. Given
pinsis closed and single-valued, the options are (i) accept aschemas/decision/**change and drop thegit diff schemas/ == 0goal, or (ii) scope the multi-capability report todoctoroutput and arming-refusal reasons only, never the DecisionRecord. Option (ii) is chosen for v1, with its cost stated plainly rather than hidden: a capability gap that blocks a merge leaves no trace in the DecisionRecord beyond the existing singlecapabilityGapstring. Recording it in the record's open top-level object is rejected — a safety-bearing field that no schema validates and no consumer must read is a fail-closed guarantee in name only. Revisiting (i) is av1alpha2conversation.
Ordering is normative: (5) and (8) are decided in this ADR; (1) and (2) before (3) and (4); (6) and (7) land with the port; and all of it before the first GitHub API call.
Consequences¶
Easier. A second adapter is TDD-able against an executable contract on day one. Capability
gaps, and therefore every arming refusal, mean one thing across forges. The audit's
"unprobed mitigations" pattern (SEC-01/SEC-04/RELI-03) gets a structural answer for new
capabilities: unprobed is unknown, and unknown does not arm. (The two github-deferred
catalog rows are not unblocked by any of this — they are L3 live-infrastructure proofs,
gated on E10-S18.)
The unknown == absent adoption cliff, stated rather than discovered. The adversarial
review established, and this ADR accepts, that the rule has teeth in both directions. Two
capabilities plausibly report unknown on GitHub forever: protected-pipeline-source —
ADR-0015 §4 makes protected-config the load-bearing arming prerequisite, and GitHub has no
single readable analogue of ci_config_path — and eligible-approval-evidence, since
the dossier §2 records that no API returns the computed per-PR eligible code owners. Under
unknown ⇒ never arm, a GitHub adapter that comments but never gates is a plausible
shipped outcome, and every fail-closed test would be green while it happened.
This ADR does not resolve that by loosening the rule — loosening it reproduces the SEC-04
pattern exactly, where a heuristic (strings.Contains(path, "@")) stood in for verification
and a pull_request_target workflow could arm on attacker-controlled config. It resolves it
by requiring, before the capability enum is frozen, a written operationally decidable
predicate for every flag: what concrete, probeable condition makes it supported on each
forge. A tri-state with no decidable membership test is a vocabulary, not a model. Where no
such predicate exists, the honest outcome is that the capability is unavailable on that forge
and the gate cannot be armed there — a product limitation to state in the docs, never to
paper over.
Harder. Five stories land before any GitHub behaviour. Every capability the GitLab adapter currently probes informally must be restated as an explicit report entry, which will surface capabilities it does not actually probe — that surfacing is the point, but it may turn GitLab arming paths that pass today into honest capability gaps. Any such change is a user-visible behaviour change and must be recorded as its own decision row, not absorbed silently into E10.
We commit to: cmd/assent never importing a concrete adapter; the conformance suite
being the only definition of forge-correct behaviour; unknown == absent for arming.
Reversible how: the port is internal (internal/forge), not public API — no
apiVersion implications and no compatibility window. Reverting means re-inlining the
composite interface at the call site and deleting the capability model; the conformance
extraction would be kept regardless, as it is a pure test-architecture improvement.
Counterpoints considered¶
"Option A is how you actually learn the seam — a port designed against one adapter is a guess." The first draft answered this by pointing at the dossier: P1-E3-S03 studied GitHub's real behaviour without writing adapter code, so the seam is informed by evidence rather than GitLab-plus-optimism.
The adversarial review broke that answer, and the correction is kept here rather than quietly edited away. The dossier is an endpoint study, not an addressing study. Every P0 above — fork-head addressing (5), status→sentinel collapse (6), eleven gaps in a single-valued field (8) — is a representation failure the dossier structurally could not surface, because naming and representation are not properties of an API surface. "Informed by real GitHub semantics" was true of the endpoints and false of the model.
The "cheap to be wrong" claim needed the same correction. It holds for a signature tweak. It
does not hold for (5), which propagates through run.go's decide/mrFrom/buildDesired/
run_render.go and collides with the byte-identical-golden requirement — that is not a
refactor inside the epic, it is a substantial part of the epic.
Why Option C still wins anyway: the review's findings are an argument for deciding more up front, not less. Each P0 was found by reading the port against the code — exactly what a seam-first epic forces someone to do — and every one of them would otherwise have been found by a GitHub adopter, in production, on a fork PR. What changes is not the option but its price: the addressing and representation model is now decided in this ADR (items 5–8) and gated by a written design note before story zero, rather than being discovered at S07.
"The capability model is speculative generality." It would be, at one adapter. At two it
is the difference between one fail-closed guarantee and two coincidentally similar ones, and
the audit already found three live cases (SEC-01, SEC-04, RELI-03) where an unprobed setting
was cited as a safety argument. unknown == absent converts that class of defect from a
per-adapter bug into a port-level impossibility.