From f7ec1248442949dce1a7b2a5381337d0de65698c Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 15:23:30 +0200 Subject: [PATCH 1/8] :memo: docs(decisions): unlock E10 + E11 (D-140/D-141) and record ADR-0021 E10 was Locked under D-012 and reaffirmed locked by D-017 and D-019, so the operator instruction to implement the GitHub adapter is a new unlock event, not the exercise of an existing one; it is recorded before any spec text exists (AGENTS.md rule 6). D-141 lifts D-017's per-rule evidence gate on the Rego backend without waiving the design need behind it. ADR-0021 decides the three things the 2026-08-09 audit found under-scoped (ARCH-18/ARCH-19): the named forge.RunPort composite port, an importable conformance suite, a neutral capability model in which unknown never arms, and port-level transport policy. The original finding text is not in the repo, so the two design buckets are recorded as a re-derivation, not a citation. --- docs/adr/0021-multi-adapter-forge-seam.md | 126 ++++++++++++++++++ docs/adr/README.md | 1 + docs/decisions/decisions.md | 2 + .../design-notes/e10-forge-port-lift.md | 33 ++++- 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 docs/adr/0021-multi-adapter-forge-seam.md diff --git a/docs/adr/0021-multi-adapter-forge-seam.md b/docs/adr/0021-multi-adapter-forge-seam.md new file mode 100644 index 00000000..8a45469b --- /dev/null +++ b/docs/adr/0021-multi-adapter-forge-seam.md @@ -0,0 +1,126 @@ +# 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: + +1. **The composite port does not exist.** AUD-S15 lifted `MRInfo` and `ErrNotFound` into + `internal/forge` (`port.go`), but `cmd/assent`'s `forgePort` is still an anonymous + interface literal declared at the call site, and `run.go` still calls + `gitlab.SyntheticDigest` directly. `port.go`'s own scope note records both as E10 work. +2. **The conformance suite cannot be reused.** All ~1,166 lines of + `internal/forge/conformance` live in `_test.go` files, 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, and + `catalog.yaml`'s `github-deferred` rows could never be flipped by construction. +3. **Capability vocabulary is adapter-private** (audit bucket A). `docs/planning/forge-dossier-github.md` + §4 enumerates eleven capability flags the port needs; `probeCapabilities` reads three + project fields, and `capabilityGap` is 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". +4. **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. All four 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: + +1. **`forge.RunPort`** — a *named* composite interface in `internal/forge`, replacing + `cmd/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)`. + `cmd/assent` depends on `forge.RunPort` **only** — a depguard rule denies + `cmd/assent` importing `internal/forge/gitlab` **and** `internal/forge/github`, replacing + the current three-symbol allowlist. The merge-digest *scheme* is adapter-owned: + `gitlab.SyntheticDigest` call-sites collapse onto `Snapshot.Heads.MergeResultDigest`. + +2. **An importable conformance package.** The case bodies move from + `internal/forge/conformance/*_test.go` into importable Go (`RunSuite(t, Factory)` over a + `forge.RunPort` factory), leaving thin `_test.go` entry points per adapter. `catalog.yaml` + remains the index and gains the adapter dimension. A case is the *same* case on both + forges or it is not a conformance case. + +3. **A neutral capability model.** `forge.Capability` is a closed, port-owned enum seeded + from dossier §4's eleven flags; adapters return a `forge.CapabilityReport` of + `supported | absent | unknown` per capability with an adapter-supplied reason. + `capabilityGap` is computed **at the port** from that report, never by an adapter, and + `unknown` is treated exactly as `absent` for arming (**unprobed is not proof**). This is + the port-level statement of ADR-0015 §4's "refuses to arm when it cannot verify". + +4. **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. + +Ordering is normative: (1) and (2) before (3) and (4), and all four before the first GitHub +API call. + +## Consequences + +**Easier.** A second adapter is TDD-able against an executable contract on day one. The +`github-deferred` catalog rows become flippable by running the same suite. `capabilityGap`, +and therefore every arming refusal, means 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. + +**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."** This is the strongest argument, and it is why the *dossier* exists: P1-E3-S03 +already studied GitHub's real behaviour (review lifecycle, GraphQL-only thread resolution, +merge queue as merge-result pin, dismissal restrictions) without writing adapter code, and +§4 explicitly records the port-design consequences. The seam is therefore informed by real +GitHub semantics, not by GitLab plus optimism. The residual risk is real but bounded, and +the mitigation is ordering, not faith: `forge.RunPort` is `internal/`, so if S07–S12 prove a +port assumption wrong, the port changes in the same epic that found the problem — at the +cost of a refactor, never a compatibility break. + +**"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. diff --git a/docs/adr/README.md b/docs/adr/README.md index 0d73ac95..0435f3f4 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -32,3 +32,4 @@ supersessions by ADR-0016/0017 are noted on each ADR's status line (not full | [0018](0018-policy-lifecycle-phase-profile-comparison.md) | Policy lifecycle — phase, profiles, comparison | Accepted (D-030) | | [0019](0019-publication-marker-reconciliation-protocol.md) | Publication marker + reconciliation protocol (database-free) | Accepted (D-030) — **one MUST unmet: doctor omits `duplicate_prevention:`** | | [0020](0020-forge-snapshot-changed-file-completeness.md) | Forge snapshot changed-file completeness contract | Accepted (D-119) | +| [0021](0021-multi-adapter-forge-seam.md) | Multi-adapter forge seam — `forge.RunPort`, neutral capabilities, transport policy | Proposed (D-140) — governs E10 | diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index 244fc1fb..767bc0af 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -144,3 +144,5 @@ project/process decisions. | D-137 | 2026-08-09 | **REL-14 — `cliff.toml` groups by the CONVENTIONAL TYPE after the gitmoji shortcode, not by the emoji; a real hotfix was published under "Other".** The parser list matched eight shortcodes (`:sparkles: :bug: :memo: :recycle: :white_check_mark: :lock: :wrench: :tada:`) and, as alternatives inside the same rules, conventional types anchored at the START of the subject (`^fix`, `^ci`, `^docs`, …). This project always writes the shortcode FIRST, so **those `^type` alternatives could never fire** — they were dead regex from the day the file was written — and every subject whose shortcode was outside the eight fell through the `.*` catch-all into **Other**. Measured on the rendered v0.2.0 Release body: `:ambulance: fix(forge): skip malformed bot markers with a warning instead of bricking reconcile (AUD-S12, REL-06)` — a fix an adopter would go looking for under **Fixes** — sat in Other, next to 18 `ci(...)` commits, 26 `test(...)`, 6 `feat(...)`, 5 `style(...)`, 4 `specs(...)`, 3 `docs(...)`, 2 `refactor(...)` and 2 `chore(...)`. **Fix:** eight new parser entries, placed AFTER the eight shortcode rules and BEFORE the `.*` catch-all, keyed on `^:[a-z0-9_]+: [(:]` — the type the author declared — plus one shortcode alias, `:test:` → Testing (a typo'd shortcode; no such gitmoji exists, and all 19 uses are tests, including one whose type field is the equally typo'd `render(...)`). Placement is deliberate and narrow: putting the type tier FIRST would re-file large parts of the eight mapped shortcodes too; placing it after means it only sorts what the catch-all was already catching. **Keyed on type, never on the emoji, because the emoji is the unreliable half** — `:lipstick: fix(provider): …` is a fix, not a UI change, and `:art:` is used for both `style(…)` and `refactor(…)`; a mapping by emoji dictionary would have mis-filed both. The inventory was derived from `git log --format=%s` over the FULL history (30 distinct shortcodes, 22 of them unmapped), not from a sample, and **every** unmapped-shortcode subject in this repo turned out to declare a conventional type, which is what makes type-keying total rather than lucky. **Judgment calls, stated because a wrong group is worse than Other:** (a) `specs(…)` → Documentation — spec authoring under `openspec/` is a written artifact, the same class the existing `:memo:|^docs` rule files; (b) `style(…)` → Refactoring — internal code hygiene with no behaviour change, which is what that group already means, and closer than Other; (c) `revert(…)` → **left in Other on purpose**: no existing group fits it and adding a Reverts group is a changelog-structure change, not a categorisation fix; (d) one malformed subject, `:test(release): add CI audit gate for single CodeQL workflow`, declares no parseable type and stays in Other — the commit is published history and cannot be reworded. Those two lines are the entire remaining Other. **Effect on already-published sections, stated as a MULTISET because the raw line diff is misleading:** the rendered bullet multiset is **identical** before and after — 509 unique bullets, 514 rendered lines, nothing added, nothing dropped — and 69 unique bullets move, all in one direction, all out of Other: Testing 28, Chores 20, Documentation 7, Refactoring 7, Features 5, Fixes 2. Zero bullets move between two non-Other groups. This re-files lines inside the already-published `[0.1.0]` and `[0.0.0]` sections; acceptable on the same ground D-136 established and the review accepted — `CHANGELOG.md` is a derived artifact regenerated in full from history, and the published v0.1.0 GitHub Release body is a separate immutable artifact that is unaffected. **Proof, both polarities, in the already-wired `release-changelog-gate-test`:** `hack/release/changelog_gate_test.sh` §8 asserts the `:ambulance:` hotfix renders under Fixes and — structurally, so it keeps holding as history grows — that NO line under Other declares a type this repo files; §8a strips the `# REL-14`-tagged entries (mutation proven to have landed by line count), shows the hotfix falls back to Other, and asserts the rendered subject multiset is unchanged by the re-grouping, so the parsers can only re-file and never add or drop a line. §7a's "changes nothing else" claim was restated over the same multiset in this lane, because removing a group's last member also removes its `###` heading and a raw line diff reads that as unexplained churn. Revert: drop the `# REL-14` entries and §8/§8a — the hotfix returns to Other. | | D-138 | 2026-08-09 | **The three reliability P1s of the 2026-08-09 audit (RELI-01/02/03) are DEFERRED past v0.2.0, deliberately and with the deferral recorded (AGENTS.md rule 6).** All three are **pre-existing at v0.1.0**; nothing in v0.2.0 introduced or worsened any of them, verified against `1d8aa60` (`origin/main` at audit time). Holding the tag would delay two fail-open fixes and a P0 in order to fix defects that are already live. **The machine direction holds in all three:** no probed path merges anything unevaluated — `MergeCAS` re-checks all three pins atomically, arming stays default-deny (`internal/forge/precondition.go`), and the `.assent/**` self-edit guard still yields BLOCK with zero forge writes. What fails is the **human signal channel**, which is why they are P1 and not P0. **RELI-01 — clean exit-0 runs leave a stale APPROVE banner, and D-130's compensating control does NOT apply here.** `cmd/assent/run.go` emits the DecisionRecord at step 8 and only then enters the step-9 switch, whose two GUARD branches (`reservedSelfEditBlock`, `untrustedExecutionContext`) skip `forge.Reconcile` **entirely** — including the summary upsert. So run 1 APPROVEs and posts `✅ Decision: APPROVE`; run 2, after a `.assent/**` edit is added, decides BLOCK, exits **0**, and leaves that note byte-identical. `openspec/specs/p5-e5-provider-host/spec.md` REQ-E5-S08-03 accepts a stale banner on the abort path precisely because "a REVIEW rerun upserts that same summary to REVIEW and adds an unresolved discussion" — the discussion being what holds the merge under `only_allow_merge_if_all_discussions_are_resolved`. **On the GUARD-1 self-edit path no thread is posted at all**, so nothing holds the merge and no later run repairs the banner: the compensating control the spec relies on is absent exactly where it is needed. Reachability is ordinary CI cadence, and it is deliberately triggerable at the cost of two pushes — D-042's own threat model rendering as a green tick. **Its fix is out of release scope BY CONSTRUCTION**, not by preference: `openspec/specs/p5-aud-audit-remediation/spec.md` pins "the decision is BLOCK with zero forge writes (GUARD-1 dominance over the gap-degrade)", so upserting a summary on that path REOPENS a frozen acceptance criterion and needs an openspec change proposal first (see OQ-31). **RELI-02 — a duplicated summary comment is UNRECOVERABLE WITHOUT A HUMAN.** `reconcileThread` has both `repairDuplicates` and a step-9 rescan; `reconcileSummary` (`internal/forge/forge.go`) has neither — it is a bare `UpsertComment`. No branch of `Reconcile` can remove a second summary note, so once two exist the wrong one can sit at `decision=APPROVE` forever. Not a corner case: `PreconditionFromCapabilities` seeds `DuplicatePrevention` to `unserialized-best-effort` because per-MR `resource_group` serialization is never probed, i.e. the race is the project's **declared default posture**. First item of v0.2.1. **RELI-03 — the standing bot approval has no retraction and the cited mitigation is never probed.** `reconcileApproveMerge` records `Approve` before `MergeCAS`; on CAS failure in the TOCTOU window the approval is already written and `forge.Forge` has no `Unapprove` verb. The code's own safety argument named the forge's remove-approvals-on-push setting — and **`reset_approvals_on_push` appears in ZERO Go files**: nothing reads it, `probeCapabilities` does not fetch it, `assent doctor` cannot report it. `spike-secure-setup.md` D11 specified refuse-to-arm when it is off and C19 specified doctor verification; **neither was built**, and the comment's deferral pointer named P4-E1-S10 — a slice that SHIPPED (D-041), so the concern was never picked up. **The comment is corrected in this lane** (`internal/forge/forge.go`, text only, no behaviour change) because an asserted-and-unbuilt mitigation TERMINATES THE SEARCH — it is how this survived two prior audits. RELI-03 needs no open question: D11 already decided it; it is unbuilt, not undecided. **Also recorded here, same shape — the ADR-0019 `duplicate_prevention:` MUST is unmet.** The value is computed and typed through to `PreconditionReport` and then never printed: `emitDoctorReport` emits only the arm verdict and refusal reasons, so no `assent doctor` output contains the string. The safe-default half DOES hold (nothing can claim `single-writer-serialized` without the mechanism). **Judgment call: annotate, do not emit.** Emitting is ~3 lines, but it is one instance of audit **ARCH-11** — doctor computes a typed capability report and prints essentially none of it — so emitting this one field would half-close ARCH-11 and leave the report inconsistent with itself, and it is a user-visible CLI output change, which does not belong in a docs-truth lane before a tag. ADR-0019 now carries the unmet-MUST annotation and points here; the emission belongs to the v0.2.1 ARCH-11 slice with its own tests. **Not deferred silently:** all three plus ARCH-11 are named in the v0.2.1 wave. Revert: `git revert` this lane — restores the false RELI-03 comment and changes no behaviour, since the lane changes none. | | D-139 | 2026-08-09 | **The security lens's SEC-01/SEC-04/SEC-05 are KNOWN and DEFERRED to v0.2.1; the tag proceeds, but the release must not claim `--checkout` is now safe.** All three are pre-existing at v0.1.0 and none was introduced by v0.2.0 (verified at `1d8aa60`). Reach on SEC-01 is bounded and that is why it does not block: **no Go non-test code constructs `--checkout`**, no CI template, example, hack script or e2e passes it, and neither `README.md` nor `docs/usage/walkthrough.md` mentions it at all — the adoption path this project actually teaches is checkout-less. The fix is also monotone-safer to ship after the tag, since the P0 already merged in this release was likewise `--checkout`-only. **SEC-01 — the `--checkout` tree is never bound to the evaluated SHA.** With `--checkout` the local tree is the sole authority for the judged bytes and the changed-file set (D-077), while `pins.sourceSha`/`targetSha` and the compare-and-swap come from the forge; **nothing compares the two**. Measured: `cmd/assent/checkout.go` contains zero occurrences of `SHA`/`Sha`, and `run.go` has no step relating the tree to a commit. Reproduced by the lens: forge head a destructive shrink, local checkout a benign grow → `APPROVE`, `approvals=1 merges=1`. ADR-0015 §2 promises every write re-verifies what it acts on; the guard re-verifies that the *metadata* has not moved, never that the judged *bytes* are the bytes at that SHA. **Condition attached and DISCHARGED IN THIS LANE:** this release hardens `--checkout` extensively (D-133 refuses any symlink anywhere) and `docs/usage/cli.md` already named "run without `--checkout`" as remedy #1, so the page read as though the flag had been made sound. A *Known limitation: the checkout is not bound to the evaluated commit* section now says otherwise, framed as a property of how the checkout is CONSTRUCTED (build `head/` from the MR head SHA; cancel superseded pipelines) and **claiming no mitigation on assent's part** — `auto_cancel_redundant_pipelines` is a project setting the tool never probes, and citing an unread setting as a control is the exact pattern this audit found three times. **Named fix for v0.2.1:** bind the checkout to the evaluated SHA, or cross-check the local changed-file set against the already-fetched `snapshot.ChangedFiles` and fold divergence opaque. **SEC-04 — protected-config verification is a substring test.** `internal/forge/gitlab/snapshot.go` sets `caps.ProtectedPipelineExternal = strings.Contains(proj.CIConfigPath, "@")`, while ADR-0015 §4 calls protected config THE load-bearing adoption prerequisite and says doctor refuses to arm when it cannot verify it. Reproduced: an in-repo, author-editable `.ci/pipeline@v2.yml` arms and merges. **Named fix:** replace the substring test with a structural parse — require a non-empty suffix after `@` that contains `/`. **SEC-05 — absent provenance is read as trusted.** `snapshot.go` computes `ForkMR: mrResp.SourceProjectID != 0 && mrResp.SourceProjectID != mrResp.ProjectID`, so an absent or zero `source_project_id` yields `ForkMR=false` and the fork guard never engages; reproduced to `approvals=1 merges=1`. **Named fix:** treat absent or zero `source_project_id` as **fork** (untrusted) — unknown provenance must fail toward advisory-only. **The composition worth not losing, recorded AS A HYPOTHESIS, not as measured:** SEC-05 × SEC-03 would let a fork MR reach the write path and then leave a **standing bot approval on a fork MR**, converting ADR-0015 §8's "CI on fork / untrusted-contributor MR → advisory-only, no writes" into "we can leave an approval on a fork". There is **no evidence real GitLab ever omits `source_project_id`**; the composition is untested and is written down so the v0.2.1 SEC-05 fix is understood as closing more than a provenance nicety. **Correction carried from the lens:** its SEC-08 was **WITHDRAWN as non-novel** — D-130 already documents the host-declaration `continue`-on-any-error as a knowingly-deferred sibling. It is not a new finding and must not be written up as one. | +| D-140 | 2026-08-10 | **E10 (GitHub adapter + Actions entrypoint) is UNLOCKED by direct operator instruction; ADR-0021 governs the seam and `openspec/specs/p5-e10-github-forge/spec.md` decomposes it into 18 stories.** E10 was **Locked** under D-012 ("unlocks with a named consumer"), and that lock was reaffirmed twice — D-017 named the consumer and still said "E10/E13 stay locked", D-019 restated "GitHub + remote packs locked". No prior decision authorized it, so the operator's instruction this session is a NEW unlock event, recorded here BEFORE any spec text or code exists (AGENTS.md rule 6). **What this authorizes**: the GitHub forge adapter, the multi-adapter seam work it depends on, and forge selection in `run`/`doctor`. **What it does NOT authorize**: E13 remote packs (still Locked under D-012 — the "named consumer" reasoning is untouched for that tier), E12 `serve`, E14 CRD, or any third forge / plugin-forge protocol (ADR-0021 Option D, rejected — no named consumer). **Scope of v1 GitHub**: behavioural parity for the GATE, not device-for-device parity (dossier §3, OQ-7/OQ-18 — required-conversation-resolution carries acknowledgement, `REQUEST_CHANGES` reserved for block); the three known deltas (review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, and where GitHub cannot prove what the gate needs the adapter reports the gap and NEVER arms — the same shape as GitLab free tier. **Why a seam epic and not just an adapter**: the 2026-08-09 audit's ARCH-18/ARCH-19 found `docs/planning/design-notes/e10-forge-port-lift.md` under-scopes the epic, and that the conformance suite cannot be run by a second adapter because all ~1,166 lines live in `_test.go` files Go cannot import — so a GitHub adapter written today would be developed against no executable contract and D-084's `github-deferred` catalog rows would be unflippable by construction. The original ARCH-18/ARCH-19 finding text is NOT in the repo (only the one-line summary at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives); the two design buckets were therefore RE-DERIVED as (A) no neutral capability model — the GitHub dossier §4 names eleven flags the port needs while `probeCapabilities` reads three project fields and `capabilityGap` is computed in GitLab terms — and (B) no port-level transport/auth policy — GitHub thread resolution is GraphQL-only and needs PAT-vs-App auth, while bounded reads/pagination caps (AUD-S10) and idempotent-GET retry/deadlines (AUD-S11) live inside the GitLab client. Both are recorded as a re-derivation, not as a citation. **OPEN SUB-QUESTION (operator)**: whether the Actions entrypoint (E10-S16) stays in scope — `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", but the action is packaging on top of an adapter and is the one story whose absence leaves everything else useful; default is to keep it last and independently droppable. **Consequence to watch (E10-S04)**: forcing every capability to be stated explicitly may turn a GitLab arming path that passes today into an honest capability gap — a user-visible behaviour change that gets its OWN decision row and changelog entry, never absorbed silently into "E10 refactor". Revert: re-lock E10 in `later-phases.md`, delete the spec directory; nothing here is published API (`forge.RunPort` is `internal/`), so no compatibility window applies. | +| D-141 | 2026-08-10 | **E11 (Rego complex-rule backend) implementation is UNLOCKED by direct operator instruction; `openspec/specs/p5-e11-rego-backend/spec.md` decomposes it into 13 stories under ADR-0002 v2.** E11's CONTRACT was already unlocked by D-017; what was gated was IMPLEMENTATION, twice: "after Phase 4" (satisfied — the Phase-4 adoption gate closed with D-042) and, per D-017, **evidence-based per rule** ("each ported rule tries CEL first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling"). This row records the operator lifting that per-rule evidence gate. **What it does NOT waive**: the DESIGN need the gate was protecting — E11-S01 still requires a written tier-1 ceiling document with concrete rules per shape (multi-pass, cross-manifest, set-difference, graph-relationship), and any shape found CEL-expressible is struck from scope. **What it does NOT authorize**: WASM or gRPC predicate backends (still Locked under D-012 — this unlocks Rego only); domain-aware joins and in-process Go rule plugins (D-017 DECLINED both permanently — not deferred); giving Rego any control over aggregation, effects, or points (ADR-0002 v2 boundary); any `EvaluationInput` change. **Two constraints found during design that shape the epic**: (1) **E11 is the first epic whose DoD is `git diff schemas/` != 0.** P3-E1-S02's backend-neutrality guarantee ("no field naming a predicate backend anywhere in the schema") applies to `EvaluationInput` and HOLDS — no decision contract changes — but `schemas/policy/v1alpha1/merge-policy.schema.json` defines the predicate leaf as `additionalProperties:false, required:["cel"]`, so a `rego:` leaf IS a policy-schema change. `API_STABILITY.md:19` permits exactly this within `v1alpha1` as an announced additive change with an openspec change and no `apiVersion` bump; the change is backward-compatible and deliberately forward-INcompatible (an older binary rejects a `rego:` leaf by strict-decode, which is the correct direction — it must not silently ignore a rule it cannot evaluate). A reviewer applying the previous epics' `git diff schemas/ == 0` habit will flag the correct change as a violation; E11-S02-04 scopes the drift guard rather than deleting it. (2) **The reflexive safety measure violates AGENTS.md rule 7.** Rego ships `time.now_ns()`, `rand.intn()`, and `http.send()`, all of which are denied structurally by the D-013 capability sandbox (E11-S04, compile-time failure, golden allowlist so an OPA upgrade cannot widen it unnoticed) — but bounding evaluation with a WALL-CLOCK TIMEOUT would itself be a rule-7 violation, making the same policy over the same ChangeSet decide differently on a slow runner. E11-S06 therefore requires a machine-independent evaluation budget, and exceeding it must be a PROCESS ERROR that can never be a policy outcome; "timeout → BLOCK" is explicitly rejected as machine-dependent while merely LOOKING fail-closed. **Also fixed by the spec**: zero violations NEVER proves a required obligation (`later-phases.md`'s explicit polarity rule; E11-S07-02 tests the failing polarity). **OPEN SUB-QUESTION (operator)**: `github.com/open-policy-agent/opa` is a large dependency with a large transitive tree on a project shipping cosign/SLSA provenance, `govulncheck`, and Scorecard — recommended default is accept-and-pin (a hand-rolled evaluator would be far worse), but it materially changes binary size and vulnerability surface and is flagged for explicit ack. Revert: re-assert the D-017 per-rule evidence gate, delete the spec directory, leave the `# locked: D-012` quarantine marker in place. | diff --git a/docs/planning/design-notes/e10-forge-port-lift.md b/docs/planning/design-notes/e10-forge-port-lift.md index 13db7f4c..1eed22b2 100644 --- a/docs/planning/design-notes/e10-forge-port-lift.md +++ b/docs/planning/design-notes/e10-forge-port-lift.md @@ -1,8 +1,39 @@ # Design note: forge port lift (pre-GitHub-adapter) — seeds E10 -Status: note only (no decision taken; decide via ADR when the GitHub adapter epic opens). +Status: **SUPERSEDED as the design authority by ADR-0021** (2026-08-10), which took the +decision this note deferred. E10 opened with D-140; the epic is +`openspec/specs/p5-e10-github-forge/spec.md`. This note is retained as the record of the +pre-AUD-S15 problem and of steps 1–2, which shipped. Trigger: ARCH-02, PROJECT-AUDIT-2026-08-06. +> **This note is INCOMPLETE as an epic scope — that was a finding, not an omission you should +> work around.** The 2026-08-09 audit recorded ARCH-18/ARCH-19: this note "under-scopes the +> epic by two design buckets, and the conformance suite cannot be run by a second adapter +> because all ~1,155 lines live in `_test.go` files Go cannot import." +> +> The original ARCH-18/ARCH-19 finding text is **not in the repo** — only the one-line summary +> at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives. The two buckets were therefore +> **re-derived** during the 2026-08-10 design session, and are recorded as a re-derivation, +> not as a citation: +> +> - **Bucket A — capability model.** `docs/planning/forge-dossier-github.md` §4 enumerates +> eleven capability flags the port needs; `probeCapabilities` reads three project fields, +> and `capabilityGap` is computed in GitLab terms. Arming (ADR-0015 §4) hangs off that +> vocabulary, so a second adapter would restate it or silently arm under a different meaning +> of "capable". ADR-0021 §3 resolves this: a port-owned capability enum, `supported | +> absent | unknown`, gap computed at the port, and **`unknown` never arms**. +> - **Bucket B — transport and auth policy.** Bounded reads and pagination caps (AUD-S10), +> idempotent-GET retry/backoff and deadlines (AUD-S11) were built into +> `internal/forge/gitlab`. GitHub additionally needs a **GraphQL** client (thread resolution +> is GraphQL-only, dossier §4) and **two auth shapes** (PAT, App installation token). Left +> at the adapter, the forges' availability and fail-closed behaviour diverge undetected. +> ADR-0021 §4 makes these port requirements with conformance cases, while leaving protocol +> and auth as adapter-internal freedom. +> +> The unimportable conformance suite is tracked separately as **E10-S01, story zero** +> (ADR-0021 §2) — until it is fixed, no adapter can be developed against an executable +> contract and D-084's `github-deferred` catalog rows are unflippable by construction. + Progress: **steps 1 and 2 below shipped in AUD-S15** (`internal/forge/port.go`, `internal/forge/port_test.go`, the ARCH-02 section of `hack/lint/depguard_test.sh`). Steps 3–5 remain open for E10. The "Problem" paragraph therefore describes the PRE-AUD-S15 state From 7e097ea6264b8ac433d610c550336421dfb10d5b Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 15:23:38 +0200 Subject: [PATCH 2/8] :memo: docs(openspec): decompose E10 and E11 into INVEST stories E10 gets 18 stories (S01-S17 autonomous, S18 infra-gated) with normative ordering: the seam lands before the first GitHub API call, because all ~1,166 lines of the conformance suite live in _test.go files Go cannot import, so an adapter written today would have no executable contract and D-084's github-deferred catalog rows would be unflippable by construction. E11 gets 13 stories. Two constraints found during design are recorded in the spec rather than discovered during implementation: it is the first epic whose DoD is 'git diff schemas/' != 0 (the rego: leaf is an announced additive change to merge-policy.schema.json; EvaluationInput is untouched, so P3-E1-S02's neutrality guarantee holds), and bounding evaluation with a wall-clock timeout would itself violate rule 7, so the budget must be machine-independent and exceeding it is a process error rather than a policy outcome. --- docs/planning/meta-plan.md | 9 +- openspec/specs/backlog.md | 57 ++- openspec/specs/later-phases.md | 28 +- openspec/specs/p5-e10-github-forge/spec.md | 550 +++++++++++++++++++++ openspec/specs/p5-e11-rego-backend/spec.md | 454 +++++++++++++++++ 5 files changed, 1089 insertions(+), 9 deletions(-) create mode 100644 openspec/specs/p5-e10-github-forge/spec.md create mode 100644 openspec/specs/p5-e11-rego-backend/spec.md diff --git a/docs/planning/meta-plan.md b/docs/planning/meta-plan.md index 1036d2c5..75aeb1fa 100644 --- a/docs/planning/meta-plan.md +++ b/docs/planning/meta-plan.md @@ -97,9 +97,12 @@ Follow-on epics cut during Phase 5, outside the E1–E9 sequence: **EFE** (`p5-pcs-policy-comparison`, full comparison-suite runner), **AUD** (`p5-aud-audit-remediation`, post-release audit remediation). -Deferred tiers keep their own numbers and unlock only with a named consumer (D-012): -**E10** GitHub adapter, **E11** Rego backend, **E12** `serve` (HTTP API), **E13** remote -packs — see the feature-maturity table in `README.md`. +Deferred tiers keep their own numbers. **E10** GitHub adapter (**unlocked D-140**, spec +`p5-e10-github-forge`, governed by ADR-0021) and **E11** Rego backend (**implementation +unlocked D-141**, spec `p5-e11-rego-backend`) are decomposed and executable; **E12** `serve` +(contract-unlocked by D-017, not decomposed) and **E13** remote packs (**still Locked** under +D-012 — the named-consumer requirement is untouched for that tier) are not. See the +feature-maturity table in `README.md`. Ordering constraint: E7 starts early (alongside E1) because every later epic's exit gate depends on it. diff --git a/openspec/specs/backlog.md b/openspec/specs/backlog.md index 8daf3bf2..422972b2 100644 --- a/openspec/specs/backlog.md +++ b/openspec/specs/backlog.md @@ -542,6 +542,61 @@ table. Every one of the **37** 2026-08-06 finding IDs is dispositioned in > check (it is skipped on PRs today, so a regression is detected only post-merge — which is > exactly what happened at `49ba1ad`). +## P5-E10 — GitHub forge adapter + Actions entrypoint (**UNLOCKED D-140**) + +Spec: [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md) · ADR: **0021** (the seam) · +Dossier: [forge-dossier-github.md](../../docs/planning/forge-dossier-github.md). +**Ordering is normative — the seam (S01–S05) lands before the first GitHub API call**, because +the conformance suite is unimportable today and an adapter written now would have no executable +contract to satisfy. S02/S04 are core-contract and require **maintainer LGTM** (GOVERNANCE); +`/agent-loop-auto` must surface them rather than auto-merge. + +| ID | Story | Execution | Depends on | Gate contribution | +| --- | --- | --- | --- | --- | +| E10-S01 | Extract the conformance suite into an importable package (`RunSuite`) | **[autonomous]** | none | **story zero** — without it no adapter can be TDD'd | +| E10-S02 | ⚠️ `forge.RunPort` named composite port + depguard denies both adapters from `cmd` | **[autonomous · engine-grade · LGTM]** | S01 | one neutral seam; ARCH-02 cannot recur | +| E10-S03 | ⚠️ Collapse `SyntheticDigest` onto `Snapshot.Heads.MergeResultDigest` | **[autonomous · engine-grade]** | S02 | digest scheme adapter-owned; allowlist emptied | +| E10-S04 | ⚠️ Neutral capability model — `unknown` never arms | **[autonomous · engine-grade · LGTM]** | S02 | one fail-closed guarantee, not two | +| E10-S05 | Port-level transport requirements (bounded reads, caps, GET-only retry, deadlines) | **[autonomous]** | S01, S04 | availability behaviour can't diverge per adapter | +| E10-S06 | GitHub client: REST + GraphQL, PAT + App installation auth | **[autonomous]** | S05 | adapter foundation; no secret in any fixture | +| E10-S07 | GitHub Snapshot (MRInfo, ADR-0020 changed-file completeness, merge-result pin) | **[autonomous]** | S06 | absent-means-trusted closed on fork detection | +| E10-S08 | ⚠️ GitHub Resolve → `ApprovalEvidence` (author/bot excluded, dismissal-aware) | **[autonomous · engine-grade]** | S06 | unprovable eligibility ⇒ unsatisfiable | +| E10-S09 | ⚠️ GitHub capability report (11 flags; unverified ⇒ `unknown`) | **[autonomous · engine-grade]** | S04, S06 | honest gaps; exhaustiveness enforced | +| E10-S10 | ⚠️ GitHub Reconcile writes (ADR-0019 parity, GraphQL thread resolution) | **[autonomous · engine-grade]** | S07–S09 | same engine, second adapter | +| E10-S11 | ⚠️ SHA-guarded merge + deferred arming + revoke-on-push | **[autonomous · engine-grade]** | S10 | ADR-0015 §2 on GitHub | +| E10-S12 | ⚠️ Capability gaps fail closed (3 deltas, `merges == 0` proven) | **[autonomous · engine-grade]** | S11 | the polarity reviews keep finding untested | +| E10-S13 | Forge selection in `run`/`doctor`; ambiguity fails closed | **[autonomous]** | S12 | no default-to-GitLab | +| E10-S14 | Conformance parity + `github-deferred` catalog flip (D-084 dispositioned) | **[autonomous]** | S13 | no bare deferrals; both factories in CI | +| E10-S15 | Docs & maturity truth (README tier, C4, `--forge`, dossier items) | **[autonomous]** | S14 | no doc claims an `unknown` capability | +| E10-S16 | Actions entrypoint (`action.yml`, pinned binary, base-ref trust) | **[autonomous — scope-flagged]** | S15 | **D-140 open sub-question**; independently droppable | +| E10-S17 | Exit gate | **[autonomous]** | S01–S16 | **the E10 exit gate** | +| E10-S18 | Live GitHub adoption proof on a real repo (mirrors D-042) | **[infra-gated · operator]** | S17 + infra | D-012-grade evidence; not an autonomous blocker | + +## P5-E11 — Complex-rule backend: Rego predicate tier (**IMPLEMENTATION UNLOCKED D-141**) + +Spec: [p5-e11-rego-backend/spec.md](p5-e11-rego-backend/spec.md) · ADR: **0002 v2** (governing). +**Two traps recorded in D-141**: E11 is the **first epic whose DoD is `git diff schemas/` != 0** +(announced additive `rego:` leaf; `schemas/decision/**` still frozen), and a **wall-clock +evaluation timeout would itself violate rule 7** — the budget must be machine-independent and +exceeding it is a process error, never a policy outcome. S02/S04/S06/S07 require **maintainer +LGTM** (published contract + the decision path itself). Independent of E10; may run in parallel. + +| ID | Story | Execution | Depends on | Gate contribution | +| --- | --- | --- | --- | --- | +| E11-S01 | Record the tier-1 (CEL) ceiling with concrete exceeding rules | **[autonomous]** | none | **do first** — a CEL-expressible shape is struck from scope | +| E11-S02 | ⚠️ Additive `rego:` leaf in the policy schema (announced, no `apiVersion` bump) | **[autonomous · engine-grade · LGTM]** | S01 | drift guard scoped, not deleted | +| E11-S03 | Module loading from the **target ref**; compile failure is a lint hard error | **[autonomous]** | S02 | no second, laxer load path | +| E11-S04 | ⚠️ OPA capability sandbox (D-013) — denied builtins fail at **compile** | **[autonomous · engine-grade · LGTM]** | S03 | rule 7 structurally; golden allowlist | +| E11-S05 | ⚠️ Input binding to the identical `EvaluationInput` | **[autonomous · engine-grade]** | S04 | proves P3-E1-S02 neutrality empirically | +| E11-S06 | ⚠️ Deterministic evaluation budget (never wall-clock) | **[autonomous · engine-grade · LGTM]** | S05 | N≥100 identical runs; budget ≠ decision | +| E11-S07 | ⚠️ Violations → findings; **zero violations never proves an obligation** | **[autonomous · engine-grade · LGTM]** | S06 | the failing polarity is tested | +| E11-S08 | ⚠️ Aggregation boundary — module cannot set effect/points/phase | **[autonomous · engine-grade]** | S07 | ADR-0002 v2 / ADR-0007 held structurally | +| E11-S09 | `assent lint` hard errors + faithful catalogue entries | **[autonomous]** | S08 | E3 parity for the second backend | +| E11-S10 | `assent test` support + both-polarity coverage | **[autonomous]** | S08 | ADR-0014 unchanged | +| E11-S11 | Remove the `# locked: D-012` quarantine; **update** the P3-E3-S04 guard | **[autonomous]** | S10 | only E11's lane may do this | +| E11-S12 | Docs & maturity truth; retire ADR-0002's "pluggable half unbuilt" line | **[autonomous]** | S11 | nothing still calls Rego locked | +| E11-S13 | Exit gate | **[autonomous]** | S01–S12 | **the E11 exit gate** | + ## Phases 3–5 Epic paragraphs (goal, ADR constraints, exit gate, story seeds) in @@ -551,7 +606,7 @@ Epic paragraphs (goal, ADR constraints, exit gate, story seeds) in | --- | --- | --- | | 3 — Contracts first | P3-E1 schemas + contract fixture (incl. ApprovalEvidence + named-consumer fixture) · P3-E2 versioning/compat spec · P3-E3 example migration · P3-E4 lifecycle: phase/profiles/comparison (ADR-0018) · P3-E5 publication reconciliation protocol (ADR-0019) | strict end-to-end contract fixture validates (ADR-0017 §8, D-016); new ADRs 0018/0019 accepted at the freeze review | | 4 — Walking skeleton | P4-E1 (+ rerun-idempotence gate, D-017) · **P2-E4-NS (OQ-24 timed run)** · holdout adjudication (OQ-25) | L3 skeleton green + **one real repo on live MRs** (D-012); north-star wording only after timed run | -| 5 — Implementation | E1–E7 **DONE**; **E7 AUTONOMOUS COMPLETE** (S01–S05+S08, D-087); **E8 AUTONOMOUS COMPLETE** ([p5-e8-renderer/spec.md](p5-e8-renderer/spec.md), S01–S14, D-098); **E9 AUTONOMOUS COMPLETE** ([p5-e9-distribution/spec.md](p5-e9-distribution/spec.md), S01–S13, D-099–D-111 CLOSED; Homebrew Formula live; PAT rotate optional); **PCS AUTONOMOUS COMPLETE** ([p5-pcs-policy-comparison/spec.md](p5-pcs-policy-comparison/spec.md), S01–S09, **D-057 closed**, D-118); E11/E12 **unlocked** (D-017); E14 gated on Spike D; E10/E13 **locked** (D-012) | per-epic; E9 exit = tagged signed release + docs live + brew Formula (D-111); PAT rotate optional | +| 5 — Implementation | E1–E7 **DONE**; **E7 AUTONOMOUS COMPLETE** (S01–S05+S08, D-087); **E8 AUTONOMOUS COMPLETE** ([p5-e8-renderer/spec.md](p5-e8-renderer/spec.md), S01–S14, D-098); **E9 AUTONOMOUS COMPLETE** ([p5-e9-distribution/spec.md](p5-e9-distribution/spec.md), S01–S13, D-099–D-111 CLOSED; Homebrew Formula live; PAT rotate optional); **PCS AUTONOMOUS COMPLETE** ([p5-pcs-policy-comparison/spec.md](p5-pcs-policy-comparison/spec.md), S01–S09, **D-057 closed**, D-118); **E10 UNLOCKED + DECOMPOSED** (D-140, [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md), 18 stories, ADR-0021); **E11 IMPLEMENTATION UNLOCKED + DECOMPOSED** (D-141, [p5-e11-rego-backend/spec.md](p5-e11-rego-backend/spec.md), 13 stories); E12 **contract-unlocked** (D-017), not decomposed; E14 gated on Spike D; **E13 still locked** (D-012) | per-epic; E9 exit = tagged signed release + docs live + brew Formula (D-111); PAT rotate optional | Named-consumer disposition (what unlocked, what stayed locked, and why): [docs/planning/named-consumer-compat.md](../../docs/planning/named-consumer-compat.md). diff --git a/openspec/specs/later-phases.md b/openspec/specs/later-phases.md index f5a50841..76d2b4df 100644 --- a/openspec/specs/later-phases.md +++ b/openspec/specs/later-phases.md @@ -257,12 +257,30 @@ schema drift guard; **D-112–D-117 cited.** **9 stories (9 autonomous)** — E1 without compare debt. Seeds: `examples/comparison/**`, `hack/compare/exitgate_test.sh`. -### E10 — GitHub adapter + Actions entrypoint — **Locked (D-012)** -Unlocks with a named consumer. Seam kept honest by the P1-E3-S03 dossier (REQUEST_CHANGES + -conversation-resolution parity, merge queue as merge-result pin, base-ref workflow trust) -and by the conformance suite being forge-neutral (E7). No frozen contract until unlock. +### E10 — GitHub adapter + Actions entrypoint — **UNLOCKED (D-140, 2026-08-10)** — spec: [p5-e10-github-forge](p5-e10-github-forge/spec.md) +Was Locked (D-012), reaffirmed locked by D-017 and D-019; unlocked by direct operator +instruction (D-140), not by the named-consumer trigger. Seam kept honest by the P1-E3-S03 +dossier (REQUEST_CHANGES + conversation-resolution parity, merge queue as merge-result pin, +base-ref workflow trust) and by the conformance suite being forge-neutral (E7). +**18 stories (S01–S17 autonomous, S18 infra-gated)** governed by **ADR-0021**, which decides +the three things the audit found under-scoped (ARCH-18/ARCH-19): the named `forge.RunPort` +composite port, an **importable** conformance suite (today all ~1,166 lines are in `_test.go` +files a second adapter cannot import), a neutral capability model where `unknown` never arms, +and port-level transport/auth policy. Ordering is normative: the seam (S01–S05) lands before +the first GitHub API call. v1 target is behavioural parity for the **gate**, with capability +gaps failing closed. -### E11 — Complex-rule backend (Rego) — **Unlocked (D-017), implementation after Phase 4** +### E11 — Complex-rule backend (Rego) — **IMPLEMENTATION UNLOCKED (D-141, 2026-08-10)** — spec: [p5-e11-rego-backend](p5-e11-rego-backend/spec.md) +Contract unlocked by D-017; implementation was gated twice — "after Phase 4" (satisfied, the +adoption gate closed with D-042) and D-017's **per-rule evidence** gate, which D-141 lifts by +operator instruction. The design need survives the lift: E11-S01 still writes the tier-1 +ceiling document, and any shape found CEL-expressible is struck from scope. **13 stories, all +autonomous.** Two constraints found during design: E11 is the **first epic whose DoD is +`git diff schemas/` != 0** (the `rego:` leaf is an announced additive change to +`merge-policy.schema.json` per `API_STABILITY.md`; `EvaluationInput` is untouched, so +P3-E1-S02's neutrality guarantee holds), and bounding evaluation with a **wall-clock timeout +would itself violate rule 7** — the budget must be machine-independent and exceeding it is a +process error, never a policy outcome. Original framing, unchanged: The named consumer's multi-pass / cross-manifest / set-difference / graph-relationship checks are the consumer D-012 required. Contract committed in Phase 3 (EvaluationInput stays backend-neutral, P3-E1); implementation only in the named-consumer expansion, and diff --git a/openspec/specs/p5-e10-github-forge/spec.md b/openspec/specs/p5-e10-github-forge/spec.md new file mode 100644 index 00000000..84c9f150 --- /dev/null +++ b/openspec/specs/p5-e10-github-forge/spec.md @@ -0,0 +1,550 @@ +# P5-E10 — GitHub forge adapter + Actions entrypoint + +**Epic ID / REQ prefix:** `E10` / `REQ-E10-S0n-nn`. + +**Unlock:** D-140 (2026-08-10). E10 was **Locked (D-012)** — "unlocks with a named consumer", +reaffirmed locked by D-017 and D-019. The operator unlocked it directly; D-140 records the +authority, the v1 scope, and what the unlock does **not** authorize. No spec text in this file +predates that row. + +**Problem**: assent has exactly one forge adapter, and the seam a second one plugs into is +half-built. AUD-S15 lifted `MRInfo`/`ErrNotFound` into `internal/forge/port.go`, but +`cmd/assent`'s `forgePort` is still an anonymous interface literal at the call site, `run.go` +still calls `gitlab.SyntheticDigest`, capability vocabulary is GitLab-private, transport +policy (bounded reads, pagination caps, retry, deadlines) lives inside +`internal/forge/gitlab`, and **all ~1,166 lines of `internal/forge/conformance` are in +`_test.go` files Go cannot import** — so the suite that defines "behaves like a forge" cannot +be run by a second adapter. `catalog.yaml`'s `github-deferred` rows (D-084) are therefore +unflippable by construction, not merely unimplemented. The 2026-08-09 audit recorded this as +ARCH-18/ARCH-19: `e10-forge-port-lift.md` under-scopes the epic by two design buckets +(capability model; transport/auth policy) plus the unimportable suite. ADR-0021 decides all +three; this epic executes it. + +**Key ground truth (de-risks the epic):** +- **The dossier is the spec input, not guesswork.** `docs/planning/forge-dossier-github.md` + already answers OQ-7 and OQ-18 (parity for the gate, not the device; + required-conversation-resolution carries acknowledgement; `REQUEST_CHANGES` reserved for + block) and §4 records the port-design consequences: eleven capability flags, two + GitHub-only behaviours the port must not preclude (review lifecycle submit→dismiss→ + re-request; **GraphQL-only thread resolution**), and the finding that nothing in the GitLab + precondition table is GitLab-only. +- **Reuse, don't re-invent:** `internal/forge/forge.go` (Reconcile, markers, fail-closed + errors), `internal/forge/fake`, `internal/forge/conformance` (cases exist; only their + *packaging* is wrong), P3-E5 fixtures under + `docs/contracts/p3-e5-publication-protocol/fixtures/`, and the E4 GitLab adapter as the + reference implementation of every port method. +- **Frozen schemas stay frozen:** epic DoD is **`git diff schemas/` == 0**. GitHub introduces + no new decision-contract field; `capabilityGap` and `pins.mergeResultDigest` already model + the absent-capability case (ADR-0017 §1, nullable only when the capability is absent). +- **`internal/core` stays I/O-free** (`TestCorePurity`); everything here is + `internal/forge/**` plus the `cmd/assent` edge. +- **The seam is `internal/`, not public API.** `forge.RunPort` carries no `apiVersion` and no + compatibility window; getting it wrong costs a refactor inside this epic, never a break. + +**Scope**: **Seam wave** — (S01) conformance suite extracted to an importable package; (S02) +`forge.RunPort` named composite port + depguard; (S03) `SyntheticDigest` collapse; (S04) +neutral capability model; (S05) port-level transport requirements. **Adapter wave** — (S06) +GitHub client (REST + GraphQL, PAT + App auth); (S07) Snapshot; (S08) Resolve → +`ApprovalEvidence`; (S09) GitHub capability report; (S10) Reconcile writes; (S11) SHA-guarded +merge + deferred arming; (S12) capability gaps fail closed. **Integration wave** — (S13) forge +selection in `run`/`doctor`; (S14) conformance parity + catalog flip; (S15) docs/maturity +truth; (S16) Actions entrypoint; (S17) exit gate. **Infra-gated:** (S18) live GitHub adoption +proof on a real repository. + +**Non-goals** (fenced): **Rego backend** (E11 — separate spec, unlocked by D-141); +**`serve` / webhooks / keyed per-MR lock** (E12); **remote policy packs** (E13, still Locked +per D-012 — D-140 unlocks E10 only); **CRD adapter** (E14, gated on Spike D); **a third forge +or a plugin/gRPC forge protocol** (ADR-0021 Option D, rejected — no named consumer); +**widening any frozen schema**; **fixing the audit's pre-existing GitLab findings** +(SEC-01/SEC-04/SEC-05, RELI-01/02/03 — deferred by D-138/D-139 and tracked there, not here); +**replacing the in-memory fake** (it stays the Reconcile unit-test substrate and becomes the +first `forge.RunPort` implementation). + +**ADRs**: **0021** (this epic's governing ADR — `RunPort`, capability model, transport +policy), 0005 (forge abstraction, conformance suite), 0011 (core ports), 0012 (finding +lifecycle), 0015 §2/§4/§8 (SHA-guard, protected pipeline, execution-authority matrix), 0017 +§1/§3/§7/§9 (merge-result pins, require-review evidence, Snapshot→Resolve→Reconcile, doctor +typed report), 0019 (marker/reconciliation protocol), 0020 (changed-file completeness). +**Reuse**: E4 GitLab adapter, `internal/forge/fake`, existing conformance cases, P3-E5 +fixtures, P1-E3-S03 GitHub dossier, AUD-S10/S11 transport hardening, AUD-S15 port lift. +**New**: importable conformance runner, `forge.RunPort`, `forge.Capability`/`CapabilityReport`, +`internal/forge/github`, forge selection, `action.yml`. + +**Executability**: S01–S17 **`[autonomous]`** with httptest servers (REST **and** GraphQL) and +the in-memory fake. S02/S04 additionally **`[maintainer LGTM]`** — ADR-0021 names the port and +capability model core-contract work per GOVERNANCE, and `/agent-loop-auto`'s stop conditions +already require surfacing public-API/core-contract changes rather than auto-merging them. S18 +**`[infra-gated · operator]`** (a real GitHub repository, live PRs, a token). + +**Dependency order**: S01 → S02 → {S03, S04} → S05 → S06 → {S07, S08, S09} → S10 → S11 → S12 +→ S13 → S14 → {S15, S16} → S17 → S18. **Do first: S01** — until the suite is importable, every +adapter story is written against no executable contract. + +## Judgment calls (decide-and-log / operator) + +(a) **🟡 OPERATOR — Actions entrypoint (S16) is IN scope but LAST and independently +droppable.** `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", so +dropping it silently would contradict the plan; but a composite action is a *distribution* +concern (E9's domain) sitting on top of an adapter, and it is the one story whose absence +leaves everything else useful. Recommended default: keep S16 as the final pre-gate story; +if the operator prefers, cut it to a follow-on and the exit gate (S17) drops its row without +any other change. **Recorded as D-140's open sub-question.** + +(b) **DECIDED — v1 GitHub target is behavioural parity for the *gate*, with capability gaps +failing closed.** Per dossier §3 and OQ-7/OQ-18: required-conversation-resolution carries +acknowledgement, `REQUEST_CHANGES` is reserved for block, and the three known deltas +(review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, not as +special cases. Where GitHub cannot prove what the gate needs, the adapter reports the gap and +**never arms** — the same shape as GitLab free tier (E4 judgment call (c), D-012 C6/C7). + +(c) **DECIDED — GraphQL is adapter-internal freedom.** Thread resolution is GraphQL-only on +GitHub (dossier §4). The port never names a transport, so the adapter owns both clients; the +conformance suite asserts *behaviour* (thread resolved / unresolved / superseded), never the +protocol used to achieve it. + +(d) **DECIDED — `unknown` capabilities are treated as `absent` for arming.** ADR-0021 §3. The +audit found three live cases where an unprobed forge setting was cited as a safety argument +(SEC-01 `auto_cancel_redundant_pipelines`, SEC-04 C17, RELI-03 `reset_approvals_on_push`). +Making "unprobed" a distinct, non-arming state converts that class of defect from a +per-adapter bug into a port-level impossibility. + +(e) **DECIDED — GitLab arming behaviour changes get their own decision row.** S04 forces every +capability the GitLab adapter *implicitly* assumed to be stated explicitly, which may turn an +arming path that passes today into an honest capability gap. That is the intended outcome, but +it is a user-visible behaviour change: it must be recorded as a `D-nnn` row of its own and +called out in the changelog, never absorbed silently into "E10 refactor". + +(f) **DECIDED — the live adoption proof (S18) mirrors D-042, and is not an exit-gate +blocker for the autonomous slice.** D-042 closed the D-012 GitLab adoption gate with real +MRs on a real project. GitHub deserves the same evidence, but it needs operator-provided +infrastructure; S17 gates the autonomous work, S18 records the live proof when the operator +runs it. + +--- + +## Seam wave + +### E10-S01 — Extract the conformance suite into an importable package `[autonomous]` + +- **Goal**: a second adapter can execute the *existing* forge conformance cases without + copying them. +- **Why now**: `internal/forge/conformance` is 1,166 lines across four `_test.go` files. Go + cannot import `_test.go`, so today the only way to conformance-test a new adapter is + duplication — which guarantees drift and makes D-084's `github-deferred` rows unflippable. +- **Dependencies**: none. **This is story zero.** +- **Definition of done**: case bodies live in importable Go; `go test ./internal/forge/...` + passes with **no case deleted, renamed, or weakened**; the GitLab entry point is a thin + `_test.go` calling the shared runner. + +- **REQ-E10-S01-01** — Given the conformance cases currently in `_test.go`, when the package + is restructured, then it exports `conformance.RunSuite(t *testing.T, f Factory)` where + `Factory` constructs a `forge.RunPort` plus its fixture state, and every existing case runs + through it. + - Test: `internal/forge/conformance/suite.go`, `internal/forge/conformance/suite_test.go` + - Verify: `go test ./internal/forge/conformance/...` + - Level: L1 +- **REQ-E10-S01-02** — Given extraction is a refactor, when the suite runs against the GitLab + factory, then the set of executed case IDs is **identical** to the pre-extraction set — proven + by a test that reads `catalog.yaml` and asserts every non-`github-deferred` row's `test` + field is executed, failing on any missing or extra case. + - Test: `internal/forge/conformance/catalog_test.go` + - Verify: `go test ./internal/forge/conformance/ -run TestCatalogMatchesExecutedCases` + - Level: L1 +- **REQ-E10-S01-03** — Given `catalog.yaml` indexes cases by `forge:`, when the schema of that + file is extended, then each row gains an explicit adapter list and the loader **rejects an + unknown adapter name** (strict-decode, P3-E2) rather than silently ignoring it. + - Test: `internal/forge/conformance/catalog.yaml`, `catalog_test.go` + - Verify: `go test ./internal/forge/conformance/ -run TestCatalogStrictDecode` + - Level: L1 + +### E10-S02 — `forge.RunPort` composite port + depguard `[autonomous · engine-grade · maintainer LGTM]` + +- **Goal**: `cmd/assent` depends on one named, forge-neutral interface and on no concrete + adapter package. +- **Dependencies**: S01 (so the port change is proven by an executable suite). +- **Definition of done**: `forge.RunPort` declared in `internal/forge`; `cmd/assent`'s + anonymous port literal deleted; depguard denies **both** concrete adapters from `cmd/assent`; + zero behaviour change (goldens and conformance byte-identical). + +- **REQ-E10-S02-01** — Given ADR-0021 §1, when `forge.RunPort` is declared, then it composes + `forge.Forge`, `forge.Snapshotter`, `forge.Resolver`, `Describe(project, mr string) + (forge.MRInfo, error)` and `FileAtRef(project, path, ref string) ([]byte, error)`, and + `cmd/assent` references that named type only. + - Test: `internal/forge/port.go`, `cmd/assent/run.go` + - Verify: `go build ./... && go test ./cmd/... ./internal/forge/...` + - Level: L1 +- **REQ-E10-S02-02** — Given ARCH-02's leak must not recur with a second adapter, when + `hack/lint/depguard_test.sh` runs, then it denies `cmd/assent` importing + `internal/forge/gitlab` **or** `internal/forge/github` with no symbol allowlist — replacing + the current `New`/`WithSleeper`/`SyntheticDigest` allowlist, which S03 empties. + - Test: `hack/lint/depguard_test.sh`, `.golangci.yml` + - Verify: `task lint` + - Level: L1 +- **REQ-E10-S02-03** — Given the fake is the Reconcile substrate, when `internal/forge/fake` + is updated, then it implements `forge.RunPort` **directly** (design-note step 5), making the + port — not the GitLab client — the conformance-tested thing. + - Test: `internal/forge/fake/fake.go`, `internal/forge/conformance/suite_test.go` + - Verify: `go test ./internal/forge/...` + - Level: L1 + +### E10-S03 — Collapse `SyntheticDigest` onto `Snapshot.Heads.MergeResultDigest` `[autonomous · engine-grade]` + +- **Goal**: the merge-result digest *scheme* is adapter-owned; `cmd/assent` computes no + forge-specific hash. +- **Dependencies**: S02. +- **Definition of done**: `run.go` reads the digest from the snapshot the adapter already + produced; the depguard symbol allowlist from S02 is now **empty**; DecisionRecord + `pins.mergeResultDigest` is byte-identical for GitLab on every existing golden. + +- **REQ-E10-S03-01** — Given design-note step 4, when `run.go` needs a merge-result pin, then + it uses `snapshot.Heads.MergeResultDigest` and calls no adapter digest function; the pin + stays nullable exactly when the capability is absent (ADR-0017 §1). + - Test: `cmd/assent/run.go`, `cmd/assent/run_test.go` + - Verify: `go test ./cmd/... && task check` + - Level: L1 +- **REQ-E10-S03-02** — Given this is a byte-identical refactor, when the E4/E7 goldens are + regenerated, then `git diff` over the golden corpus is **empty**. + - Test: existing golden corpus + - Verify: `task test && git diff --exit-code -- test/ examples/` + - Level: L1 + +### E10-S04 — Neutral capability model `[autonomous · engine-grade · maintainer LGTM]` + +- **Goal**: `capabilityGap` means the same thing on every forge, and unprobed never arms. +- **Dependencies**: S02. +- **Definition of done**: `forge.Capability` closed enum seeded from dossier §4's eleven + flags; `forge.CapabilityReport` returns `supported | absent | unknown` + reason per + capability; the gap is computed **at the port**; `unknown` blocks arming identically to + `absent`; `assent doctor` prints the typed report (ADR-0017 §9, ADR-0019's + `duplicate_prevention:` MUST). + +- **REQ-E10-S04-01** — Given dossier §4, when `forge.Capability` is declared, then it contains + exactly the eleven named flags (`resolvable-threads`, `threads-block-merge`, + `blocking-review`, `review-dismissal-restrictions`, `sha-guarded-merge`, + `deferred-merge-arming`, `arming-revoked-on-push`, `merge-result-pinning`, + `eligible-approval-evidence`, `approval-reset-on-push`, `protected-pipeline-source`), and + decoding an unknown capability name is an error, not a skip. + - Test: `internal/forge/capability.go`, `internal/forge/capability_test.go` + - Verify: `go test ./internal/forge/ -run TestCapabilityEnumClosed` + - Level: L0 +- **REQ-E10-S04-02** — Given ADR-0021 §3 and judgment call (d), when a capability is reported + `unknown`, then every arming decision treats it as `absent` — proven by a table test over + all three states asserting `unknown` and `absent` produce the identical non-arming outcome + and a distinguishable *reason* string. + - Test: `internal/forge/capability_test.go` + - Verify: `go test ./internal/forge/ -run TestUnknownDoesNotArm` + - Level: L1 +- **REQ-E10-S04-03** — Given `capabilityGap` is port-computed, when the GitLab adapter is + migrated to return a `CapabilityReport`, then no adapter computes a gap itself, and any + capability the adapter does not actually probe is reported `unknown` — **not** `supported`. + - Test: `internal/forge/gitlab/capability.go`, `internal/forge/gitlab/capability_test.go` + - Verify: `go test ./internal/forge/gitlab/` + - Level: L1 +- **REQ-E10-S04-04** — Given judgment call (e), when S04 changes any GitLab arming outcome + that passed before, then a `D-nnn` row records it and the changelog carries a user-facing + entry; when no outcome changes, the story records that explicitly. + - Test: `docs/decisions/decisions.md` + - Verify: manual — story cannot close without one of the two statements present + - Level: L0 + +### E10-S05 — Port-level transport requirements `[autonomous]` + +- **Goal**: availability and fail-closed behaviour are properties of the port, not of one + HTTP client. +- **Dependencies**: S01, S04. +- **Definition of done**: bounded response reads, pagination caps, idempotent-GET-only retry + with backoff, and context deadlines are stated as port requirements with conformance cases; + the GitLab adapter (AUD-S10/S11) satisfies them unchanged; **writes are never retried**. + +- **REQ-E10-S05-01** — Given AUD-S10, when any adapter reads a forge response, then the read + is byte-bounded and paginated collection reads are capped, with exhaustion failing **closed** + (never a silent truncation) — asserted by conformance cases that serve oversized and + over-paginated responses. + - Test: `internal/forge/conformance/transport.go`, `catalog.yaml` + - Verify: `go test ./internal/forge/conformance/ -run TestConformanceBoundedReads` + - Level: L1 +- **REQ-E10-S05-02** — Given AUD-S11, when a request fails transiently, then only idempotent + GETs retry (bounded, backed off, deadline-bounded) and **no write is ever retried** — + asserted by a conformance case counting write attempts across an injected 5xx. + - Test: `internal/forge/conformance/transport.go` + - Verify: `go test ./internal/forge/conformance/ -run TestConformanceWritesNeverRetried` + - Level: L1 + +## Adapter wave + +### E10-S06 — GitHub client: REST + GraphQL transports, PAT and App auth `[autonomous]` + +- **Dependencies**: S05. +- **Definition of done**: `internal/forge/github` with both transports behind adapter-internal + interfaces, httptest cassettes for both, and two auth shapes; **no token value ever logged + or embedded in an error**; the package compiles with zero imports from `cmd/assent`. + +- **REQ-E10-S06-01** — Given dossier §4, when the adapter needs thread resolution, then it + holds a GraphQL client alongside the REST client, both exercised by httptest cassettes, and + the port surface names neither. + - Test: `internal/forge/github/client.go`, `internal/forge/github/client_test.go` + - Verify: `go test ./internal/forge/github/` + - Level: L2 +- **REQ-E10-S06-02** — Given GitHub supports PAT and GitHub App installation tokens, when + credentials are supplied, then both shapes authenticate, installation-token refresh is + handled, and a missing/expired credential fails **closed** with an error naming no secret + material. + - Test: `internal/forge/github/auth.go`, `internal/forge/github/auth_test.go` + - Verify: `go test ./internal/forge/github/ -run TestAuth` + - Level: L2 +- **REQ-E10-S06-03** — Given `gitleaks` and D-002, when the adapter and its cassettes are + committed, then no real token, org name, or private repository name appears in any fixture. + - Test: `internal/forge/github/testdata/**` + - Verify: `task scrub && task check` + - Level: L0 + +### E10-S07 — GitHub Snapshot `[autonomous]` + +- **Dependencies**: S06. +- **Definition of done**: PR metadata → `forge.MRInfo` (head/base SHAs, fork detection), + changed-file enumeration satisfying **ADR-0020 completeness** (truncation is an opaque + enumeration failure, never a short list), and merge-result pinning via `refs/pull/N/merge`. + +- **REQ-E10-S07-01** — Given `forge.MRInfo`'s contract, when a PR is described, then + `SourceSHA` is the PR head, `TargetSHA` is the **base branch tip** (not the merge base), + and `ForkMR` is true iff the head repository differs from the base repository — with the + **absent-means-trusted** trap closed: an absent/`null` head-repo field yields an error, not + `ForkMR=false` (audit SEC-05's GitLab analogue). + - Test: `internal/forge/github/snapshot.go`, `snapshot_test.go` + - Verify: `go test ./internal/forge/github/ -run TestSnapshotMRInfo` + - Level: L2 +- **REQ-E10-S07-02** — Given ADR-0020, when the changed-file listing is truncated or paginated + past the cap, then Snapshot reports an **opaque enumeration failure** and the run fails + closed; a complete listing reports completeness explicitly. + - Test: `internal/forge/github/snapshot.go` + - Verify: `go test ./internal/forge/github/ -run TestChangedFilesCompleteness` + - Level: L2 +- **REQ-E10-S07-03** — Given dossier C16, when a merge-result pin is available, then + `Heads.MergeResultDigest` is derived from the merge ref, and when merge-queue or merge-ref + semantics make it unavailable, the digest is **nil with a capability gap** — never a + fabricated value. + - Test: `internal/forge/github/snapshot.go` + - Verify: `go test ./internal/forge/github/ -run TestMergeResultPin` + - Level: L2 + +### E10-S08 — GitHub Resolve → `ApprovalEvidence` `[autonomous · engine-grade]` + +- **Dependencies**: S06. +- **Definition of done**: typed `ApprovalEvidence` per dossier §2, validating against the + frozen `schemas/decision/v1alpha1/approval-evidence.schema.json` with `git diff schemas/` + == 0; PR author and bot identities excluded; dismissed reviews never count. + +- **REQ-E10-S08-01** — Given dossier §2, when reviews are resolved, then evidence is built + from the review chain (latest non-dismissed review per eligible reviewer), the **PR author + is excluded**, bot identities are excluded, and a `REQUEST_CHANGES` review is carried as + block signal — never as approval. + - Test: `internal/forge/github/resolve.go`, `resolve_test.go` + - Verify: `go test ./internal/forge/github/ -run TestResolveApprovalEvidence` + - Level: L2 +- **REQ-E10-S08-02** — Given the review lifecycle (submit → dismiss → re-request), when a + previously-approving review is dismissed or the reviewer is re-requested, then the evidence + no longer counts that approval — asserted on a cassette replaying all three transitions. + - Test: `internal/forge/github/resolve_test.go` + - Verify: `go test ./internal/forge/github/ -run TestDismissedApprovalNotCounted` + - Level: L2 +- **REQ-E10-S08-03** — Given eligibility cannot always be proven (dossier §2 plan gating), + when the eligible-approver set is unavailable, then Resolve reports a capability gap and + `require-review` is **unsatisfiable** — never satisfied by an unproven approval. + - Test: `internal/forge/github/resolve.go` + - Verify: `go test ./internal/forge/github/ -run TestUnprovableEligibilityFailsClosed` + - Level: L2 + +### E10-S09 — GitHub capability report `[autonomous · engine-grade]` + +- **Dependencies**: S04, S06. +- **Definition of done**: the adapter returns a `CapabilityReport` covering all eleven flags; + every capability it does not actually probe is `unknown`; plan/visibility gating is + reflected honestly. + +- **REQ-E10-S09-01** — Given S04's enum, when the GitHub adapter reports capabilities, then + every one of the eleven flags carries a state and a reason, and a compile-time-exhaustive + test fails if a new capability is added without a GitHub answer. + - Test: `internal/forge/github/capability.go`, `capability_test.go` + - Verify: `go test ./internal/forge/github/ -run TestCapabilityExhaustive` + - Level: L1 +- **REQ-E10-S09-02** — Given dossier "open verification items", when a capability's real + behaviour is unverified against a live API (merge-queue plan gating, `enablePullRequestAutoMerge` + preconditions), then it is reported `unknown` with the open item cited — not optimistically + `supported`. + - Test: `internal/forge/github/capability.go` + - Verify: `go test ./internal/forge/github/ -run TestUnverifiedReportedUnknown` + - Level: L1 + +### E10-S10 — GitHub Reconcile writes `[autonomous · engine-grade]` + +- **Dependencies**: S07, S08, S09. +- **Definition of done**: ADR-0019 marker protocol parity — idempotent finding threads, + duplicate repair, occurrence supersession, resolve-no-longer-desired, post-publication + rescan, summary slot — all through the shared `internal/forge` engine, with GraphQL used + for thread resolution. + +- **REQ-E10-S10-01** — Given ADR-0019, when Reconcile runs on GitHub, then the **same** + `internal/forge` engine drives it (the adapter supplies primitives only) and the S01 + conformance replay cases pass against the GitHub factory. + - Test: `internal/forge/github/reconcile_test.go`, `internal/forge/conformance/` + - Verify: `go test ./internal/forge/... -run TestConformance` + - Level: L1 +- **REQ-E10-S10-02** — Given contributor marker spoofing (E4-S09 precedent), when threads are + listed, then the author-identity filter excludes non-bot authors, and a malformed marker is + **skipped with a warning** rather than bricking reconciliation (RELI-06 precedent). + - Test: `internal/forge/github/reconcile_test.go` + - Verify: `go test ./internal/forge/github/ -run TestMarkerSpoofAndMalformed` + - Level: L2 + +### E10-S11 — SHA-guarded merge + deferred arming `[autonomous · engine-grade]` + +- **Dependencies**: S10. +- **Definition of done**: merge is SHA-pinned (ADR-0015 §2); deferred arming uses + `enablePullRequestAutoMerge`; merge queue is treated as the merge-result pin; + arming-revoked-on-push is honoured. + +- **REQ-E10-S11-01** — Given ADR-0015 §2, when head or base has moved since evaluation, then + the merge fails closed with the shared `ErrSHAMoved` — asserted by the S01 SHA-guard + conformance cases running against the GitHub factory. + - Test: `internal/forge/conformance/` (GitHub factory) + - Verify: `go test ./internal/forge/conformance/ -run TestConformanceSHAGuard` + - Level: L1 +- **REQ-E10-S11-02** — Given dossier C8′/C11/C14, when arming is requested, then + `enablePullRequestAutoMerge` is used, a subsequent push revokes the arming, and if the + revoke-on-push capability is `unknown` or `absent`, **arming is refused**. + - Test: `internal/forge/github/merge.go`, `merge_test.go` + - Verify: `go test ./internal/forge/github/ -run TestArmingRevokeOnPush` + - Level: L2 + +### E10-S12 — Capability gaps fail closed on GitHub `[autonomous · engine-grade]` + +- **Dependencies**: S11. +- **Definition of done**: each of the three known GitHub deltas (dismissal restrictions, + auto-merge revoke, merge queue) has an explicit fail-closed test proving no auto-merge when + the capability is `absent` or `unknown`. + +- **REQ-E10-S12-01** — Given judgment call (b), when any capability required by the armed path + is `absent` or `unknown`, then the DecisionRecord carries an honest `capabilityGap`, the run + does not merge, and the reason is contributor-legible in the posted comment. + - Test: `internal/forge/github/`, `cmd/assent/run_test.go` + - Verify: `go test ./... -run TestCapabilityGapBlocksMerge` + - Level: L1 +- **REQ-E10-S12-02** — Given fail-closed claims are worthless untested, when each of the three + deltas is simulated absent, then a table test asserts `merges == 0` for all three — the + polarity E6/AUD reviews repeatedly found untested. + - Test: `internal/forge/github/failclosed_test.go` + - Verify: `go test ./internal/forge/github/ -run TestDeltasFailClosed` + - Level: L1 + +## Integration wave + +### E10-S13 — Forge selection in `run` / `doctor` `[autonomous]` + +- **Dependencies**: S12. +- **Definition of done**: explicit `--forge {gitlab|github}` plus remote-host autodetect; + **ambiguity or an unrecognised host fails closed** (no default-to-GitLab); `cmd/assent` + still imports no concrete adapter (S02's depguard holds). + +- **REQ-E10-S13-01** — Given two adapters exist, when the forge is not explicitly selected and + cannot be unambiguously detected, then the run **errors** rather than defaulting — asserted + including the unknown-host and conflicting-signal cases. + - Test: `cmd/assent/forge_select.go`, `forge_select_test.go` + - Verify: `go test ./cmd/... -run TestForgeSelection` + - Level: L1 +- **REQ-E10-S13-02** — Given S02, when forge selection is wired, then construction happens + behind a factory returning `forge.RunPort`, and `task lint`'s depguard still denies + `cmd/assent` importing either adapter package. + - Test: `hack/lint/depguard_test.sh` + - Verify: `task lint` + - Level: L1 + +### E10-S14 — Conformance parity + catalog flip `[autonomous]` + +- **Dependencies**: S13. +- **Definition of done**: the S01 suite runs against the GitHub factory in CI; every + `github-deferred` row in `catalog.yaml` is either flipped to `both` or **retains the + deferral with a named, cited reason**; D-084 is dispositioned. + +- **REQ-E10-S14-01** — Given D-084, when the catalog is updated, then no row remains + `github-deferred` without a reason field naming the blocking open-verification item, and a + test fails on any bare deferral. + - Test: `internal/forge/conformance/catalog.yaml`, `catalog_test.go` + - Verify: `go test ./internal/forge/conformance/ -run TestNoBareDeferrals` + - Level: L1 +- **REQ-E10-S14-02** — Given both adapters implement `forge.RunPort`, when CI runs, then + `RunSuite` executes against **both** factories in the same job, and a new case added for one + forge fails the build until the other declares support or a cited deferral. + - Test: `internal/forge/conformance/suite_test.go`, `.github/workflows/verify.yaml` + - Verify: `task check` + - Level: L1 + +### E10-S15 — Docs and maturity truth `[autonomous]` + +- **Dependencies**: S14. +- **Definition of done**: README feature-maturity table moves GitHub from **Planned** to its + earned tier; C4 diagrams updated (ARCH-05 precedent — planned vs shipped legend); `cli.md` + documents `--forge`; dossier "open verification items" each dispositioned + (verified / still open / reported `unknown`). + +- **REQ-E10-S15-01** — Given the audit's docs-truth family, when GitHub ships, then no + document claims a GitHub capability the capability report marks `unknown` — asserted by a + docs-truth test comparing the README maturity row against the adapter's reported states. + - Test: `README.md`, `hack/docs/maturity_test.sh` + - Verify: `task check` + - Level: L1 +- **REQ-E10-S15-02** — Given `later-phases.md` and `meta-plan.md` carry E10's status, when the + epic closes, then both are updated in the same change as the exit gate, and the E10 row + cites D-140. + - Test: `openspec/specs/later-phases.md`, `docs/planning/meta-plan.md` + - Verify: manual + `task check` + - Level: L0 + +### E10-S16 — Actions entrypoint `[autonomous — scope-flagged, judgment call (a)]` + +- **Dependencies**: S15. +- **Definition of done**: a composite `action.yml` invoking the released binary at a pinned + version, an example workflow, and documentation; **no new adapter behaviour** — the action + is packaging only. + +- **REQ-E10-S16-01** — Given E9's distribution model, when the action runs, then it consumes a + **pinned, checksum-verified** released binary (never `go install` at HEAD), and the pin is + asserted by a test reading `action.yml`. + - Test: `action.yml`, `hack/release/action_pin_test.sh` + - Verify: `task check` + - Level: L1 +- **REQ-E10-S16-02** — Given ADR-0015's trust boundaries, when the action is documented, then + the workflow example uses the **base-ref workflow trust** model from the dossier (policy + loaded from the target ref, never the PR head) and states the required token scopes. + - Test: `docs/`, `action.yml` + - Verify: manual review + `task check` + - Level: L0 + +### E10-S17 — Exit gate `[autonomous]` + +- **Dependencies**: S01–S16. +- **Definition of done**: `hack/forge/e10_exitgate_test.sh` proves, in one invocation: + `RunSuite` green against **both** factories; zero bare `github-deferred` rows; `task check` + green; `git diff schemas/` == 0; depguard denies both adapters from `cmd/assent`; the + capability enum is exhaustively answered by both adapters; every fail-closed polarity test + present. If judgment call (a) drops S16, the gate drops that row and nothing else. + +- **REQ-E10-S17-01** — Given every prior story, when the exit gate runs, then it fails if any + of the above conditions regresses, and it cites D-140 plus ADR-0021. + - Test: `hack/forge/e10_exitgate_test.sh` + - Verify: `bash hack/forge/e10_exitgate_test.sh && task check` + - Level: L1 + +### E10-S18 — Live GitHub adoption proof `[infra-gated · operator]` + +- **Dependencies**: S17 + operator-provided infrastructure. +- **Definition of done**: mirroring D-042 — assent runs on **live PRs** in a real GitHub + repository, producing at least one REVIEW (with a resolvable thread) and one APPROVE with a + real SHA-pinned merge; DecisionRecords retained under + `docs/decisions/evidence/p5-e10-s18-adoption/`; a `D-nnn` row records the proof. + +- **REQ-E10-S18-01** — Given D-012's "synthetic does not count" standard, when the proof is + recorded, then the evidence names a real repository and real PR URLs, and the open + verification items resolved by the live run are moved out of `unknown` in S09's report. + - Test: `docs/decisions/evidence/p5-e10-s18-adoption/` + - Verify: operator-run; evidence committed + - Level: L3 diff --git a/openspec/specs/p5-e11-rego-backend/spec.md b/openspec/specs/p5-e11-rego-backend/spec.md new file mode 100644 index 00000000..f3c37f20 --- /dev/null +++ b/openspec/specs/p5-e11-rego-backend/spec.md @@ -0,0 +1,454 @@ +# P5-E11 — Complex-rule backend: Rego predicate tier + +**Epic ID / REQ prefix:** `E11` / `REQ-E11-S0n-nn`. + +**Unlock:** D-141 (2026-08-10). E11's *contract* was already unlocked by D-017; what was gated +was **implementation**, twice over: "after Phase 4" (satisfied — the Phase-4 adoption gate +closed with D-042) and, per D-017, **evidence-based per rule** — "each ported rule tries CEL +first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling." +D-141 records the operator lifting that per-rule evidence gate and what it does **not** waive. + +**Problem**: ADR-0002 v2 promises "one Kyverno-style YAML envelope, **pluggable expression +backends**" — and the ADR index has carried the status line "**pluggable half unbuilt: Rego is +E11**" ever since. There is exactly one backend (CEL, ADR-0013), and the CEL leaf is +restricted to the frozen predicate-scope table: single-pass, single-subject, no set +operations across manifests, no graph relationships. The escape hatch was designed and +committed — `examples/policies/rego/bounded_change.rego` exists and is *quarantined* behind a +`# locked: D-012` marker with a CI guard (P3-E3-S03/S04) forbidding any declarative example +from referencing a `rego:` leaf. E11 is the epic that removes that quarantine and makes the +second tier real. + +**Two hard constraints that shape every story:** + +1. **The authoring schema names the backend; the decision schema does not.** P3-E1-S02's + backend-neutrality guarantee (REQ-P3-E1-S02-01: "no field naming a predicate backend … + anywhere in the schema") applies to `EvaluationInput` — and it holds, so **no decision + contract changes**. But `schemas/policy/v1alpha1/merge-policy.schema.json` defines the + predicate leaf as `{"additionalProperties": false, "required": ["cel"], "properties": + {"cel": …, "message": …}}`. A `rego:` leaf therefore **requires a policy-schema change**. + `API_STABILITY.md:19` permits exactly this within `v1alpha1`: "Additive field additions + within a major require an openspec change + version bump before they become required" and + "within a major, changes are additive-only for reports and **announced-only for authored + policy**." This spec *is* that openspec change. **E11 is therefore the first epic whose DoD + is `git diff schemas/` != 0** — every prior epic required it to be zero, and a reviewer + applying the old habit will flag the correct change as a violation. + +2. **Rule 7 (determinism) is the sharpest constraint, and the obvious implementation breaks + it.** AGENTS.md rule 7 forbids anything probabilistic or wall-clock-dependent in the + decision path. Rego's standard builtins include `time.now_ns()`, `rand.intn()`, and + `http.send()` — each of which would make the same policy over the same ChangeSet produce + different decisions on different runs. Worse, the *reflexive* safety measure — "bound Rego + evaluation with a timeout" — is itself a rule-7 violation: a wall-clock deadline makes the + decision depend on machine speed and load, so a policy that passes on a fast runner blocks + on a slow one. E11 must bound evaluation **deterministically** (S06) and deny the + non-deterministic builtins **structurally** (S04), not by convention. + +**Key ground truth (de-risks the epic):** +- **The contract shape is already decided and reviewable.** ADR-0002 v2: the envelope owns + `match` / `effect` / `points`; a Rego module **only computes violations** over the policy + input. `examples/policies/rego/bounded_change.rego` is the committed, reviewed illustration + of that shape. E11 implements the decided contract; it does not redesign it. +- **`later-phases.md` fixes the boundaries**: violations-shaped modules, **explicit + obligation-proof polarity (no implicit "no violation = proof")**, OPA capability sandbox + (D-013), the same typed `EvaluationInput`, structured proof/finding output, declared data, + **no I/O**, and **no control over aggregation**. Hard boundary: no domain-aware joins, no Go + rule plugins in `internal/core`. +- **`docs/planning/rego-escape-hatch.md` names E11 as the only lane permitted to remove the + quarantine marker** — and the guard that enforces it lives in P3-E3-S04 + (`hack/check-migration-invariants.sh`). Unquarantining is a story here (S11), not a + side-effect of any other story. +- **`internal/core` stays I/O-free** (`TestCorePurity`). Module *loading* is I/O and lives at + the loader tier alongside CEL compilation; module *evaluation* is pure computation and may + live in core only once S04's sandbox makes that true by construction. + +**Scope**: (S01) the tier-1 ceiling, recorded with concrete exceeding rules; (S02) additive +`rego:` leaf in the policy schema; (S03) module loading + compile-time errors; (S04) OPA +capability sandbox (D-013); (S05) input binding to the identical `EvaluationInput`; (S06) +deterministic evaluation budget; (S07) violations → findings with explicit obligation-proof +polarity; (S08) aggregation boundary — the module never controls effect/points; (S09) +`assent lint` hard errors for Rego rules; (S10) `assent test` + goldens; (S11) remove the +quarantine + update the P3-E3-S04 guard; (S12) docs/maturity truth; (S13) exit gate. + +**Non-goals** (fenced): **GitHub adapter** (E10 — separate spec, unlocked by D-140); +**`serve`** (E12); **remote packs** (E13, still Locked per D-012); **domain-aware joins** and +**in-process Go rule plugins** (D-017 declined both, permanently — not deferred); +**giving Rego control over aggregation, effects, or points** (ADR-0002 v2 boundary); +**WASM or gRPC predicate backends** (still Locked per D-012 — D-141 unlocks Rego only); +**widening the frozen predicate-scope table for CEL**; **any `EvaluationInput` change** +(the whole point of P3-E1-S02's neutrality is that none is needed — if a story finds one +necessary, that is a design failure to surface, not a schema bump to make). + +**ADRs**: 0002 v2 (**the governing ADR** — pluggable backends; its "pluggable half unbuilt" +status line is retired by S12), 0007 (effects and aggregation — the boundary Rego must not +cross), 0013 (CEL as tier 1; the ceiling E11 sits above), 0017 §2/§3/§9 (obligations, proof +polarity, do-not-generalize list), 0018 (phase/profile — a Rego rule is phase-gated like any +other). **Related decisions**: D-012 (escape-hatch quarantine), D-013 (OPA capability +sandbox), D-017 (contract unlock + declined items), D-141 (implementation unlock). +**Reuse**: the E2 decision engine's leaf-evaluation seam, `internal/core/policy`'s strict +loader, E3's lint hard-error framework, E6's `assent test` harness, the committed example. +**New**: `rego:` leaf, OPA integration + capability file, deterministic budget, violation +mapping. + +**Executability**: S01–S13 all **`[autonomous]`** — hermetic, no infrastructure. S02, S04, +S06, S07 are **engine-grade** (frozen-schema change, sandbox, determinism, decision polarity) +and additionally **`[maintainer LGTM]`**: S02 changes a published contract and S04/S06/S07 are +the decision path itself. + +**Dependency order**: S01 → S02 → S03 → S04 → S05 → S06 → S07 → S08 → {S09, S10} → S11 → S12 +→ S13. **Do first: S01** — the ceiling document determines whether the backend's shape is +right; building it without one reproduces the speculative-generality risk D-012 existed to +prevent. + +## Judgment calls (decide-and-log / operator) + +(a) **DECIDED — the `rego:` leaf is an additive `oneOf` alternative, not a replacement.** +Every policy valid before E11 stays valid; a `rego:` leaf is rejected by an older assent +binary through strict-decode, which is the **correct** direction (an old binary must not +silently ignore a rule it cannot evaluate). This is backward-compatible and deliberately +forward-**in**compatible, and S02 records it in `API_STABILITY.md` as an announced additive +change — no `apiVersion` bump. + +(b) **DECIDED — evaluation is bounded by a deterministic budget, never a wall-clock timeout.** +Per rule 7: the bound is an OPA evaluation-step/instruction budget that yields the identical +outcome on any machine. A wall-clock deadline is permitted **only** as an outer backstop that +can never change a decision — i.e. exceeding it is a hard process error, never a policy +outcome (not a BLOCK, not an APPROVE, not a skipped rule). If that separation cannot be +implemented cleanly, the story fails and the operator is asked; it is not resolved by +"timeout → BLOCK", which would make the decision machine-dependent while *looking* fail-closed. + +(c) **DECIDED — zero violations is NOT proof of an obligation.** `later-phases.md` names this +explicitly and it is the single easiest thing to get wrong: a module that errors, is +misconfigured, matches nothing, or returns an empty set would otherwise silently *satisfy* a +required obligation. A Rego-backed obligation is proven only by an explicit, structured proof +value; absence of violations satisfies **non-obligation** rules only. S07 owns both polarities +and must test the failing one. + +(d) **🟡 OPERATOR — the OPA dependency is a supply-chain decision, not just an import.** +`github.com/open-policy-agent/opa` is a large dependency with a large transitive tree, on a +project whose release story includes SLSA-grade provenance, cosign signing, `govulncheck`, and +Scorecard. Adding it materially changes binary size, vulnerability surface, and the +`renovate`/`govulncheck` maintenance load. Recommended default: **accept**, since a +hand-rolled Rego evaluator would be far worse, and pin + vendor-audit it in S03. Flagged for +an explicit operator ack because it is the kind of change D-012's philosophy ("no speculative +frozen contracts for tiers without users") exists to make deliberate. **Recorded as D-141's +open sub-question.** + +(e) **DECIDED — Rego modules are policy, and load from the target ref like all policy.** +ADR-0010/ADR-0015's trust rules apply unchanged: a module is loaded from the target ref, +never from the PR head, so a contributor cannot ship a rule change and have it govern their +own PR. S03 must not introduce a second, laxer load path. + +(f) **DECIDED — E11 and E10 are independent and may run in either order or in parallel.** +They share no files: E10 is `internal/forge/**` + `cmd/assent` edge; E11 is +`internal/core/**` + `schemas/policy/**` + `examples/policies/rego/**`. The only coupling is +review bandwidth. Recommended sequencing is **E10 first** — it has a live-adoption story +(S18) whose infrastructure the operator must arrange, so starting it early parallelizes the +human dependency. + +--- + +### E11-S01 — Record the tier-1 ceiling with concrete exceeding rules `[autonomous]` + +- **Goal**: a written, reviewable statement of what CEL *cannot* express, grounded in real + rules — the artifact D-017's evidence gate was protecting. +- **Why first**: D-141 lifts the per-rule evidence *gate*, not the design need. The four + shapes `later-phases.md` names (multi-pass, cross-manifest, set-difference, + graph-relationship) determine what the input binding (S05) and the violation shape (S07) + must support. Building those without the ceiling document is guesswork. +- **Dependencies**: none. +- **Definition of done**: `docs/planning/rego-tier-ceiling.md` exists with ≥1 concrete, + sanitized rule per named shape, each showing the CEL attempt and *why* it fails. + +- **REQ-E11-S01-01** — Given the four shapes, when the ceiling document is authored, then each + carries a concrete generic rule, the attempted CEL leaf, and the specific reason it cannot + be expressed within the frozen predicate-scope table (`docs/planning/predicate-scope.md`) — + and no employer or internal system name appears (D-002). + - Test: `docs/planning/rego-tier-ceiling.md` + - Verify: `task scrub && task check` + - Level: L0 +- **REQ-E11-S01-02** — Given a shape might in fact be CEL-expressible, when the document is + reviewed, then any shape found expressible in CEL is **struck from E11's scope** and + recorded — the epic narrows rather than building an unjustified tier. + - Test: `docs/planning/rego-tier-ceiling.md`, `openspec/specs/p5-e11-rego-backend/spec.md` + - Verify: manual review + - Level: L0 + +### E11-S02 — Additive `rego:` leaf in the policy schema `[autonomous · engine-grade · maintainer LGTM]` + +- **Dependencies**: S01. +- **Definition of done**: `merge-policy.schema.json`'s `leaf` becomes a `oneOf` over the + existing `cel` shape and a new `rego` shape; strict-decode still rejects unknown fields and + a leaf carrying **both** backends; `API_STABILITY.md` records the announced additive change; + every pre-E11 policy fixture still validates unchanged. + +- **REQ-E11-S02-01** — Given the leaf is `additionalProperties:false, required:["cel"]`, when + the schema is extended, then a `{"rego": {...}}` leaf validates, a `{"cel": ..., "rego": ...}` + leaf is **rejected** (exactly one backend per leaf), and an unknown key is still rejected. + - Test: `schemas/policy/v1alpha1/merge-policy.schema.json`, `schemas/schema_test.go` + - Verify: `go test ./schemas/... -run TestMergePolicySchema` + - Level: L0 +- **REQ-E11-S02-02** — Given backward compatibility, when the full pre-E11 example and + fixture corpus is validated against the new schema, then **every document still validates** + and no golden changes. + - Test: `examples/**`, `test/**` + - Verify: `task test && git diff --exit-code -- examples/ test/` + - Level: L1 +- **REQ-E11-S02-03** — Given `API_STABILITY.md:19`'s "announced-only for authored policy", + when the schema changes, then `API_STABILITY.md` and the changelog record it as an announced + additive change within `v1alpha1` with **no `apiVersion` bump**, and state explicitly that an + older binary rejects a `rego:` leaf by design. + - Test: `API_STABILITY.md`, `CHANGELOG.md` + - Verify: `task check` + - Level: L0 +- **REQ-E11-S02-04** — Given every prior epic's DoD was `git diff schemas/` == 0, when E11's + gates run, then the schema-drift guard is **scoped**, not deleted: drift is permitted only + in `merge-policy.schema.json` and only for this change; drift in any `schemas/decision/**` + file still fails. + - Test: `hack/` schema-drift guard + - Verify: `task check` + - Level: L1 + +### E11-S03 — Module loading and compile-time errors `[autonomous]` + +- **Dependencies**: S02. +- **Definition of done**: modules resolve from the pack directory **on the target ref** + (judgment call (e)); a module that fails to compile is a **load-time hard error** (E3 lint + parity), never a runtime surprise; the OPA dependency is pinned. + +- **REQ-E11-S03-01** — Given ADR-0010/0015 trust rules, when a Rego module is loaded, then it + resolves through the **same target-ref policy load path** as YAML policy, and no second + loader can read a module from the PR head — asserted by a test that places a hostile module + on the head ref and proves it is not evaluated. + - Test: `internal/core/policy/rego_load.go`, `rego_load_test.go` + - Verify: `go test ./internal/core/... -run TestRegoLoadsFromTargetRef` + - Level: L1 +- **REQ-E11-S03-02** — Given E3's hard-error framework, when a module fails to compile or + references an undefined rule, then `assent lint` reports it as a **hard error** with the + file and position, and `assent run` refuses to evaluate — fail closed, never skip the rule. + - Test: `internal/core/policy/rego_load_test.go` + - Verify: `go test ./internal/core/... -run TestRegoCompileErrorIsHardError` + - Level: L1 +- **REQ-E11-S03-03** — Given judgment call (d), when OPA is added to `go.mod`, then the + version is pinned, `govulncheck` and `renovate` cover it, and the binary-size delta is + recorded in the story's notes. + - Test: `go.mod`, `go.sum` + - Verify: `task check && govulncheck ./...` + - Level: L0 + +### E11-S04 — OPA capability sandbox `[autonomous · engine-grade · maintainer LGTM]` + +- **Dependencies**: S03. +- **Definition of done**: D-013's sandbox is real — a capability set that **denies by + default** and allows an explicit, enumerated builtin list; `http.send`, `net.*`, + `opa.runtime`, `time.*`, `rand.*`, and any I/O builtin are unavailable; a module using one + fails to **compile**, not at runtime. + +- **REQ-E11-S04-01** — Given rule 7 and D-013, when a module calls `http.send`, `net.lookup_ip_addr`, + `time.now_ns`, `rand.intn`, or `opa.runtime`, then compilation **fails** with a message + naming the denied builtin — one test case per denied builtin, each asserting failure. + - Test: `internal/core/policy/rego_capabilities.go`, `rego_capabilities_test.go` + - Verify: `go test ./internal/core/... -run TestDeniedBuiltins` + - Level: L1 +- **REQ-E11-S04-02** — Given allowlists rot silently, when the allowed builtin set changes, + then a test comparing the effective set against a **committed golden list** fails — so + adding a builtin is a deliberate, reviewed act, and an OPA upgrade that introduces new + builtins cannot widen the sandbox unnoticed. + - Test: `internal/core/policy/testdata/allowed-builtins.golden` + - Verify: `go test ./internal/core/... -run TestAllowedBuiltinsGolden` + - Level: L1 +- **REQ-E11-S04-03** — Given `TestCorePurity`, when Rego evaluation lives in `internal/core`, + then the purity test still passes and the sandbox is what makes that true by construction — + if evaluation cannot be made pure, it moves out of core rather than weakening the test. + - Test: `internal/core/` purity test + - Verify: `go test ./internal/core/... -run TestCorePurity` + - Level: L1 + +### E11-S05 — Input binding: the identical `EvaluationInput` `[autonomous · engine-grade]` + +- **Dependencies**: S04. +- **Definition of done**: a Rego module sees the same typed input a CEL leaf sees, proving + P3-E1-S02's neutrality claim empirically; **no `EvaluationInput` schema change**. + +- **REQ-E11-S05-01** — Given REQ-P3-E1-S02-01, when a rule is evaluated by either backend, + then both receive input derived from the **same** `EvaluationInput` instance — asserted by a + test that evaluates an equivalent rule under both backends and compares the bound input. + - Test: `internal/core/policy/rego_input_test.go` + - Verify: `go test ./internal/core/... -run TestBackendsShareInput` + - Level: L1 +- **REQ-E11-S05-02** — Given typed facts (Spike C / OQ-17), when a fact is `unavailable`, + `invalid`, or `expired`, then the module observes that **typed state** and cannot mistake it + for a value — an unavailable fact must not read as absent-and-therefore-fine (the + "absent-means-trusted" pattern the 2026-08-09 audit found three times). + - Test: `internal/core/policy/rego_input_test.go` + - Verify: `go test ./internal/core/... -run TestUnavailableFactVisibleToModule` + - Level: L1 +- **REQ-E11-S05-03** — Given `git diff schemas/decision/` must stay empty, when S05 lands, + then no decision-contract file has changed. + - Test: `schemas/decision/**` + - Verify: `git diff --exit-code -- schemas/decision/` + - Level: L0 + +### E11-S06 — Deterministic evaluation budget `[autonomous · engine-grade · maintainer LGTM]` + +- **Dependencies**: S05. +- **Definition of done**: evaluation is bounded by a machine-independent budget; exceeding it + can **never** produce a policy outcome (judgment call (b)); rule-7 determinism is proven by + repeated evaluation. + +- **REQ-E11-S06-01** — Given rule 7, when the same module evaluates the same input N times + (N ≥ 100), then the output — violations, their order, and their messages — is **byte-identical + every time**, including on a machine under load. + - Test: `internal/core/policy/rego_determinism_test.go` + - Verify: `go test ./internal/core/... -run TestRegoDeterminism -count=1` + - Level: L1 +- **REQ-E11-S06-02** — Given judgment call (b), when the evaluation budget is exceeded, then + the run fails with a **process error**, and a test asserts the outcome is neither APPROVE nor + BLOCK nor a silently-skipped rule — the failure mode is "assent could not decide", not a + decision. + - Test: `internal/core/policy/rego_budget_test.go` + - Verify: `go test ./internal/core/... -run TestBudgetExceededIsNotADecision` + - Level: L1 +- **REQ-E11-S06-03** — Given Go map iteration order is random, when violations are emitted, + then they are canonically sorted before entering the decision path — the same defect class + the audit found in `internal/change/diff_hcl.go` and `entries.go`. + - Test: `internal/core/policy/rego_eval.go` + - Verify: `task determinism` + - Level: L1 + +### E11-S07 — Violations → findings, with explicit obligation-proof polarity `[autonomous · engine-grade · maintainer LGTM]` + +- **Dependencies**: S06. +- **Definition of done**: a module's violation set maps to findings with `EntryRef` subjects; + **zero violations never proves an obligation** (judgment call (c)); both polarities tested. + +- **REQ-E11-S07-01** — Given ADR-0002 v2's shape, when a module returns violations, then each + maps to a finding carrying the rule, subject (`EntryRef`), and message, validating against + the frozen `DecisionRecord` schema with no schema change. + - Test: `internal/core/policy/rego_findings.go`, `rego_findings_test.go` + - Verify: `go test ./internal/core/... -run TestRegoViolationsToFindings` + - Level: L1 +- **REQ-E11-S07-02** — Given `later-phases.md`'s explicit polarity rule, when a Rego-backed + **required obligation** yields zero violations **without** an explicit structured proof, + then the obligation is **NOT satisfied** and the decision is not APPROVE — asserted by a + test whose module returns an empty violation set and whose expected outcome is + *unsatisfied*. + - Test: `internal/core/policy/rego_polarity_test.go` + - Verify: `go test ./internal/core/... -run TestEmptyViolationsDoNotProveObligation` + - Level: L1 +- **REQ-E11-S07-03** — Given a module can be broken in ways that resemble success, when a + module is undefined, returns a non-set value, or produces a malformed violation, then each + case fails **closed** — three distinct test cases, each asserting `approve == false`. + - Test: `internal/core/policy/rego_polarity_test.go` + - Verify: `go test ./internal/core/... -run TestMalformedModuleFailsClosed` + - Level: L1 + +### E11-S08 — Aggregation boundary `[autonomous · engine-grade]` + +- **Dependencies**: S07. +- **Definition of done**: the envelope owns `match`, `effect`, and `points`; a module cannot + influence any of them, and the boundary is enforced structurally rather than by convention. + +- **REQ-E11-S08-01** — Given ADR-0002 v2 and ADR-0007, when a module emits a value that + *looks* like an effect or a points value (e.g. `{"effect": "block", "points": 99}`), then it + is **ignored**: the finding's effect and points come from the envelope, proven by a test + whose module tries to escalate and whose expected outcome is the envelope's. + - Test: `internal/core/policy/rego_boundary_test.go` + - Verify: `go test ./internal/core/... -run TestModuleCannotSetEffectOrPoints` + - Level: L1 +- **REQ-E11-S08-02** — Given ADR-0018, when a Rego-backed rule carries a `phase`, then the + same never-additive phase ceiling applies as for CEL rules (`CoverWithPhaseCeiling`) — a + Rego rule is not a phase bypass. + - Test: `internal/core/policy/rego_boundary_test.go` + - Verify: `go test ./internal/core/... -run TestRegoRuleRespectsPhaseCeiling` + - Level: L1 + +### E11-S09 — `assent lint` hard errors for Rego rules `[autonomous]` + +- **Dependencies**: S08. +- **Definition of done**: E3's six hard-error checks have Rego equivalents where meaningful, + plus Rego-specific ones (missing module, undefined entry rule, denied builtin, both-backends + leaf); the rule catalogue (`assent catalogue`) reports Rego-backed rules faithfully. + +- **REQ-E11-S09-01** — Given E3's framework, when a pack contains a broken Rego rule, then + `assent lint` exits non-zero with a positioned, contributor-legible message per failure + class — one test per class. + - Test: `internal/core/lint/rego_test.go` + - Verify: `go test ./internal/core/lint/` + - Level: L1 +- **REQ-E11-S09-02** — Given D-048's catalogue rules, when a Rego-backed rule is catalogued, + then its entry is faithful (authored `phase`, `effectivePhase`, generated `docs.url`) and + fabricates no lifecycle metadata. + - Test: `internal/core/catalogue/` + - Verify: `go test ./internal/core/catalogue/` + - Level: L1 + +### E11-S10 — `assent test` support and goldens `[autonomous]` + +- **Dependencies**: S08. +- **Definition of done**: an adopter can test a Rego-backed rule with the same + `expect.yaml` / `cases.yaml` contract (ADR-0014, no schema change); `--coverage` counts Rego + rules in **both** polarities. + +- **REQ-E11-S10-01** — Given ADR-0014, when a Rego-backed rule is exercised by `assent test`, + then the expectation format is unchanged and both a violating and a non-violating case are + covered. + - Test: `examples/` test fixtures + - Verify: `assent test ./examples/... && task check` + - Level: L1 +- **REQ-E11-S10-02** — Given E6's both-polarity coverage rule, when `--coverage` runs, then a + Rego rule counts as covered only when **both** polarities are exercised. + - Test: `internal/core/testharness/` + - Verify: `go test ./internal/core/...` + - Level: L1 + +### E11-S11 — Remove the quarantine `[autonomous]` + +- **Dependencies**: S10. +- **Definition of done**: per `docs/planning/rego-escape-hatch.md` ("Only **E11's own + implementation lane** … may remove the marker"), the `# locked: D-012` marker is removed + from `examples/policies/rego/**`, the P3-E3-S04 guard is **updated rather than deleted**, + and the examples enter the schema-validation CI job and the golden corpus. + +- **REQ-E11-S11-01** — Given the guard asserts the marker's presence, when the marker is + removed, then `hack/check-migration-invariants.sh` is updated so it still forbids what + remains forbidden (no `rego:` leaf in an archetype/starter pack that has not been migrated) + and no longer asserts a marker that must not exist — the guard is never simply deleted. + - Test: `hack/check-migration-invariants.sh` + - Verify: `task check` + - Level: L1 +- **REQ-E11-S11-02** — Given the example was excluded from CI, when the quarantine lifts, then + `examples/policies/rego/bounded_change.rego` validates, compiles under S04's capabilities, + and is exercised by `assent test`. + - Test: `examples/policies/rego/bounded_change.rego` + - Verify: `task check` + - Level: L1 + +### E11-S12 — Docs and maturity truth `[autonomous]` + +- **Dependencies**: S11. +- **Definition of done**: README feature-maturity moves Rego from **Locked** to its earned + tier; **ADR-0002's index status line "pluggable half unbuilt: Rego is E11" is retired**; + `docs/planning/rego-escape-hatch.md` records that the quarantine was lifted by E11-S11 and + by what authority; predicate-scope docs state which backend each restriction applies to. + +- **REQ-E11-S12-01** — Given the audit's docs-truth family, when E11 ships, then no document + describes Rego as locked, quarantined, or unbuilt, and none claims a capability S01 struck + from scope. + - Test: `README.md`, `docs/adr/README.md`, `docs/planning/rego-escape-hatch.md` + - Verify: `task check` + - Level: L1 + +### E11-S13 — Exit gate `[autonomous]` + +- **Dependencies**: S01–S12. +- **Definition of done**: `hack/policy/e11_exitgate_test.sh` proves in one invocation: denied + builtins fail compilation (all cases); determinism over N ≥ 100 runs; the empty-violations + polarity test present and failing-closed; the effect/points boundary held; the scoped schema + drift confined to `merge-policy.schema.json`; `schemas/decision/**` unchanged; the example + unquarantined and green; `task check` green. + +- **REQ-E11-S13-01** — Given every prior story, when the exit gate runs, then it fails on any + regression above and cites D-141 plus ADR-0002 v2. + - Test: `hack/policy/e11_exitgate_test.sh` + - Verify: `bash hack/policy/e11_exitgate_test.sh && task check` + - Level: L1 From dca5ef2bbc4561405bf09dcf227b18bc6975caf9 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 15:34:04 +0200 Subject: [PATCH 3/8] :memo: docs(openspec): fix two P0 design defects found by adversarial review An independent roast of the first draft read the port against the code and found two representation defects that the endpoint-level GitHub dossier could not have surfaced. Fork-PR head addressing: run.go:274 reads head content by branch name inside one project and forge.MRInfo carries no source-repository identifier, so a GitHub fork PR 404s, fileAtRefOrAbsent maps that to nil, and OneSidedLifecycle returns KindDelete -- every fork PR would be evaluated as a whole-file deletion the contributor never made. The port now addresses content relative to the merge request and a conformance case pins it. Capability record surface: $defs.pins is additionalProperties:false with a single-string capabilityGap required iff mergeResultDigest is null, so an eleven-capability report has nowhere valid to live. ADR-0021 scopes the report to doctor output and arming-refusal reasons and states the audit-trail cost rather than widening a frozen schema or hiding the field in an unvalidated one. Also corrected: S02 could not close (cmd/assent constructs gitlab.New, and depguard_test.sh hard-fails unless it sees that call-site, while task lint is S02's own Verify) -- the neutral factory moves from S13 into S02 and the scanner's positive control is replaced rather than deleted. Every merges==0 assertion gains a paired merges==1 positive control, because an adapter that never arms would otherwise satisfy all of them. S14 now requires an adapter disposition on every catalog row, not only the deferred ones, which would otherwise have shipped GitHub with zero trust-boundary cases proven. Two false claims are retracted in place: the github-deferred rows are L3 live-infra proofs that importability cannot unblock, and capabilityGap never modelled the general absent-capability case. --- docs/adr/0021-multi-adapter-forge-seam.md | 152 +++++++++++-- docs/decisions/decisions.md | 2 +- openspec/specs/backlog.md | 28 ++- openspec/specs/p5-e10-github-forge/spec.md | 234 ++++++++++++++++++--- openspec/specs/p5-e11-rego-backend/spec.md | 102 +++++++-- 5 files changed, 439 insertions(+), 79 deletions(-) diff --git a/docs/adr/0021-multi-adapter-forge-seam.md b/docs/adr/0021-multi-adapter-forge-seam.md index 8a45469b..1c50feec 100644 --- a/docs/adr/0021-multi-adapter-forge-seam.md +++ b/docs/adr/0021-multi-adapter-forge-seam.md @@ -16,11 +16,21 @@ seam a second adapter must plug into is only half-built: `internal/forge` (`port.go`), but `cmd/assent`'s `forgePort` is still an anonymous interface literal declared at the call site, and `run.go` still calls `gitlab.SyntheticDigest` directly. `port.go`'s own scope note records both as E10 work. -2. **The conformance suite cannot be reused.** All ~1,166 lines of +2. **The conformance suite cannot be reused.** All 1,166 lines of `internal/forge/conformance` live in `_test.go` files, 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, and - `catalog.yaml`'s `github-deferred` rows could never be flipped by construction. + 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 unblock `catalog.yaml`'s `github-deferred` rows. **That was false** — + both such rows are `level: 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.Forge` internals + (`sha_guard_test.go:49` takes `*fake.Forge`; `reconciliation_test.go:220` type-asserts to + it), so a `Factory` returning a `forge.RunPort` is 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. 3. **Capability vocabulary is adapter-private** (audit bucket A). `docs/planning/forge-dossier-github.md` §4 enumerates eleven capability flags the port needs; `probeCapabilities` reads three project fields, and `capabilityGap` is computed in GitLab terms. Arming decisions @@ -35,8 +45,46 @@ seam a second adapter must plug into is only half-built: 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. All four must be decided before adapter code, because each one changes what the -adapter is written *against*. +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: + +5. **Head-content addressing — the port cannot read a GitHub fork PR's head (P0).** + `cmd/assent/run.go:270,274` reads the governed subject's base and head via + `FileAtRef(project, path, ref)` with `info.TargetBranch` / `info.SourceBranch` — a **branch + name inside one project** — and `forge.MRInfo` carries no source-repository identifier. On + GitHub, a fork PR's head branch does not exist in the base repo, so the read 404s; + `fileAtRefOrAbsent` (run.go:465-468) maps `forge.ErrNotFound` to `nil`, and + `change.OneSidedLifecycle(base, nil)` (`internal/change/onesided.go:20-21`) returns + `KindDelete, 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. + Freezing `FileAtRef`'s signature at story 2 without deciding this is the single largest + latent refactor in the epic. +6. **Error taxonomy at the port.** `forge.ErrNotFound` is not a transport code — it is a + **semantic presence signal** consumed by `fileAtRefOrAbsent` and turned into a `FileEvent`. + 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. +7. **Identity at the port.** `RunPort` carries no identity; the GitLab adapter smuggles + `botAuthor` through 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 a `User`, an App's is a bot). +8. **Record surface for multi-capability gaps (P0).** `schemas/decision/v1alpha1/decision-record.schema.json` + defines `$defs.pins` with `additionalProperties: false` and a **single string** + `capabilityGap`, required *iff* `mergeResultDigest` is `null` (and forbidden otherwise, via + an `if/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's `git diff schemas/ == 0` goal 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 @@ -81,16 +129,68 @@ consisting of four committed pieces: installation token) and protocol (REST vs. GraphQL) stay **adapter-internal freedom** — the port never names a transport. -Ordering is normative: (1) and (2) before (3) and (4), and all four before the first GitHub -API call. +5. **An explicit addressing model, decided before `FileAtRef` is frozen.** The port stops + addressing content 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/head` on + 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. Smuggling + `refs/pull/N/head` into `MRInfo.SourceBranch` is explicitly rejected: it corrupts a + documented field and leaks into rendering. + *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. + +6. **A per-adapter HTTP-status → port-sentinel mapping, with a conformance case per sentinel.** + `ErrNotFound` means *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. + +7. **Identity is a port concept.** `RunPort` exposes 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). + +8. **The capability report's record surface is decided here, not in a story.** Given `pins` is + closed and single-valued, the options are (i) accept a `schemas/decision/**` change and + drop the `git diff schemas/ == 0` goal, or (ii) scope the multi-capability report to + `doctor` output 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 single + `capabilityGap` string.* 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 a `v1alpha2` conversation. + +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. The -`github-deferred` catalog rows become flippable by running the same suite. `capabilityGap`, -and therefore every arming refusal, means one thing across forges. The audit's +**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. +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 @@ -110,14 +210,28 @@ extraction would be kept regardless, as it is a pure test-architecture improveme ## Counterpoints considered **"Option A is how you actually learn the seam — a port designed against one adapter is a -guess."** This is the strongest argument, and it is why the *dossier* exists: P1-E3-S03 -already studied GitHub's real behaviour (review lifecycle, GraphQL-only thread resolution, -merge queue as merge-result pin, dismissal restrictions) without writing adapter code, and -§4 explicitly records the port-design consequences. The seam is therefore informed by real -GitHub semantics, not by GitLab plus optimism. The residual risk is real but bounded, and -the mitigation is ordering, not faith: `forge.RunPort` is `internal/`, so if S07–S12 prove a -port assumption wrong, the port changes in the same epic that found the problem — at the -cost of a refactor, never a compatibility break. +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 diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index 767bc0af..c5635b61 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -145,4 +145,4 @@ project/process decisions. | D-138 | 2026-08-09 | **The three reliability P1s of the 2026-08-09 audit (RELI-01/02/03) are DEFERRED past v0.2.0, deliberately and with the deferral recorded (AGENTS.md rule 6).** All three are **pre-existing at v0.1.0**; nothing in v0.2.0 introduced or worsened any of them, verified against `1d8aa60` (`origin/main` at audit time). Holding the tag would delay two fail-open fixes and a P0 in order to fix defects that are already live. **The machine direction holds in all three:** no probed path merges anything unevaluated — `MergeCAS` re-checks all three pins atomically, arming stays default-deny (`internal/forge/precondition.go`), and the `.assent/**` self-edit guard still yields BLOCK with zero forge writes. What fails is the **human signal channel**, which is why they are P1 and not P0. **RELI-01 — clean exit-0 runs leave a stale APPROVE banner, and D-130's compensating control does NOT apply here.** `cmd/assent/run.go` emits the DecisionRecord at step 8 and only then enters the step-9 switch, whose two GUARD branches (`reservedSelfEditBlock`, `untrustedExecutionContext`) skip `forge.Reconcile` **entirely** — including the summary upsert. So run 1 APPROVEs and posts `✅ Decision: APPROVE`; run 2, after a `.assent/**` edit is added, decides BLOCK, exits **0**, and leaves that note byte-identical. `openspec/specs/p5-e5-provider-host/spec.md` REQ-E5-S08-03 accepts a stale banner on the abort path precisely because "a REVIEW rerun upserts that same summary to REVIEW and adds an unresolved discussion" — the discussion being what holds the merge under `only_allow_merge_if_all_discussions_are_resolved`. **On the GUARD-1 self-edit path no thread is posted at all**, so nothing holds the merge and no later run repairs the banner: the compensating control the spec relies on is absent exactly where it is needed. Reachability is ordinary CI cadence, and it is deliberately triggerable at the cost of two pushes — D-042's own threat model rendering as a green tick. **Its fix is out of release scope BY CONSTRUCTION**, not by preference: `openspec/specs/p5-aud-audit-remediation/spec.md` pins "the decision is BLOCK with zero forge writes (GUARD-1 dominance over the gap-degrade)", so upserting a summary on that path REOPENS a frozen acceptance criterion and needs an openspec change proposal first (see OQ-31). **RELI-02 — a duplicated summary comment is UNRECOVERABLE WITHOUT A HUMAN.** `reconcileThread` has both `repairDuplicates` and a step-9 rescan; `reconcileSummary` (`internal/forge/forge.go`) has neither — it is a bare `UpsertComment`. No branch of `Reconcile` can remove a second summary note, so once two exist the wrong one can sit at `decision=APPROVE` forever. Not a corner case: `PreconditionFromCapabilities` seeds `DuplicatePrevention` to `unserialized-best-effort` because per-MR `resource_group` serialization is never probed, i.e. the race is the project's **declared default posture**. First item of v0.2.1. **RELI-03 — the standing bot approval has no retraction and the cited mitigation is never probed.** `reconcileApproveMerge` records `Approve` before `MergeCAS`; on CAS failure in the TOCTOU window the approval is already written and `forge.Forge` has no `Unapprove` verb. The code's own safety argument named the forge's remove-approvals-on-push setting — and **`reset_approvals_on_push` appears in ZERO Go files**: nothing reads it, `probeCapabilities` does not fetch it, `assent doctor` cannot report it. `spike-secure-setup.md` D11 specified refuse-to-arm when it is off and C19 specified doctor verification; **neither was built**, and the comment's deferral pointer named P4-E1-S10 — a slice that SHIPPED (D-041), so the concern was never picked up. **The comment is corrected in this lane** (`internal/forge/forge.go`, text only, no behaviour change) because an asserted-and-unbuilt mitigation TERMINATES THE SEARCH — it is how this survived two prior audits. RELI-03 needs no open question: D11 already decided it; it is unbuilt, not undecided. **Also recorded here, same shape — the ADR-0019 `duplicate_prevention:` MUST is unmet.** The value is computed and typed through to `PreconditionReport` and then never printed: `emitDoctorReport` emits only the arm verdict and refusal reasons, so no `assent doctor` output contains the string. The safe-default half DOES hold (nothing can claim `single-writer-serialized` without the mechanism). **Judgment call: annotate, do not emit.** Emitting is ~3 lines, but it is one instance of audit **ARCH-11** — doctor computes a typed capability report and prints essentially none of it — so emitting this one field would half-close ARCH-11 and leave the report inconsistent with itself, and it is a user-visible CLI output change, which does not belong in a docs-truth lane before a tag. ADR-0019 now carries the unmet-MUST annotation and points here; the emission belongs to the v0.2.1 ARCH-11 slice with its own tests. **Not deferred silently:** all three plus ARCH-11 are named in the v0.2.1 wave. Revert: `git revert` this lane — restores the false RELI-03 comment and changes no behaviour, since the lane changes none. | | D-139 | 2026-08-09 | **The security lens's SEC-01/SEC-04/SEC-05 are KNOWN and DEFERRED to v0.2.1; the tag proceeds, but the release must not claim `--checkout` is now safe.** All three are pre-existing at v0.1.0 and none was introduced by v0.2.0 (verified at `1d8aa60`). Reach on SEC-01 is bounded and that is why it does not block: **no Go non-test code constructs `--checkout`**, no CI template, example, hack script or e2e passes it, and neither `README.md` nor `docs/usage/walkthrough.md` mentions it at all — the adoption path this project actually teaches is checkout-less. The fix is also monotone-safer to ship after the tag, since the P0 already merged in this release was likewise `--checkout`-only. **SEC-01 — the `--checkout` tree is never bound to the evaluated SHA.** With `--checkout` the local tree is the sole authority for the judged bytes and the changed-file set (D-077), while `pins.sourceSha`/`targetSha` and the compare-and-swap come from the forge; **nothing compares the two**. Measured: `cmd/assent/checkout.go` contains zero occurrences of `SHA`/`Sha`, and `run.go` has no step relating the tree to a commit. Reproduced by the lens: forge head a destructive shrink, local checkout a benign grow → `APPROVE`, `approvals=1 merges=1`. ADR-0015 §2 promises every write re-verifies what it acts on; the guard re-verifies that the *metadata* has not moved, never that the judged *bytes* are the bytes at that SHA. **Condition attached and DISCHARGED IN THIS LANE:** this release hardens `--checkout` extensively (D-133 refuses any symlink anywhere) and `docs/usage/cli.md` already named "run without `--checkout`" as remedy #1, so the page read as though the flag had been made sound. A *Known limitation: the checkout is not bound to the evaluated commit* section now says otherwise, framed as a property of how the checkout is CONSTRUCTED (build `head/` from the MR head SHA; cancel superseded pipelines) and **claiming no mitigation on assent's part** — `auto_cancel_redundant_pipelines` is a project setting the tool never probes, and citing an unread setting as a control is the exact pattern this audit found three times. **Named fix for v0.2.1:** bind the checkout to the evaluated SHA, or cross-check the local changed-file set against the already-fetched `snapshot.ChangedFiles` and fold divergence opaque. **SEC-04 — protected-config verification is a substring test.** `internal/forge/gitlab/snapshot.go` sets `caps.ProtectedPipelineExternal = strings.Contains(proj.CIConfigPath, "@")`, while ADR-0015 §4 calls protected config THE load-bearing adoption prerequisite and says doctor refuses to arm when it cannot verify it. Reproduced: an in-repo, author-editable `.ci/pipeline@v2.yml` arms and merges. **Named fix:** replace the substring test with a structural parse — require a non-empty suffix after `@` that contains `/`. **SEC-05 — absent provenance is read as trusted.** `snapshot.go` computes `ForkMR: mrResp.SourceProjectID != 0 && mrResp.SourceProjectID != mrResp.ProjectID`, so an absent or zero `source_project_id` yields `ForkMR=false` and the fork guard never engages; reproduced to `approvals=1 merges=1`. **Named fix:** treat absent or zero `source_project_id` as **fork** (untrusted) — unknown provenance must fail toward advisory-only. **The composition worth not losing, recorded AS A HYPOTHESIS, not as measured:** SEC-05 × SEC-03 would let a fork MR reach the write path and then leave a **standing bot approval on a fork MR**, converting ADR-0015 §8's "CI on fork / untrusted-contributor MR → advisory-only, no writes" into "we can leave an approval on a fork". There is **no evidence real GitLab ever omits `source_project_id`**; the composition is untested and is written down so the v0.2.1 SEC-05 fix is understood as closing more than a provenance nicety. **Correction carried from the lens:** its SEC-08 was **WITHDRAWN as non-novel** — D-130 already documents the host-declaration `continue`-on-any-error as a knowingly-deferred sibling. It is not a new finding and must not be written up as one. | | D-140 | 2026-08-10 | **E10 (GitHub adapter + Actions entrypoint) is UNLOCKED by direct operator instruction; ADR-0021 governs the seam and `openspec/specs/p5-e10-github-forge/spec.md` decomposes it into 18 stories.** E10 was **Locked** under D-012 ("unlocks with a named consumer"), and that lock was reaffirmed twice — D-017 named the consumer and still said "E10/E13 stay locked", D-019 restated "GitHub + remote packs locked". No prior decision authorized it, so the operator's instruction this session is a NEW unlock event, recorded here BEFORE any spec text or code exists (AGENTS.md rule 6). **What this authorizes**: the GitHub forge adapter, the multi-adapter seam work it depends on, and forge selection in `run`/`doctor`. **What it does NOT authorize**: E13 remote packs (still Locked under D-012 — the "named consumer" reasoning is untouched for that tier), E12 `serve`, E14 CRD, or any third forge / plugin-forge protocol (ADR-0021 Option D, rejected — no named consumer). **Scope of v1 GitHub**: behavioural parity for the GATE, not device-for-device parity (dossier §3, OQ-7/OQ-18 — required-conversation-resolution carries acknowledgement, `REQUEST_CHANGES` reserved for block); the three known deltas (review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, and where GitHub cannot prove what the gate needs the adapter reports the gap and NEVER arms — the same shape as GitLab free tier. **Why a seam epic and not just an adapter**: the 2026-08-09 audit's ARCH-18/ARCH-19 found `docs/planning/design-notes/e10-forge-port-lift.md` under-scopes the epic, and that the conformance suite cannot be run by a second adapter because all ~1,166 lines live in `_test.go` files Go cannot import — so a GitHub adapter written today would be developed against no executable contract and D-084's `github-deferred` catalog rows would be unflippable by construction. The original ARCH-18/ARCH-19 finding text is NOT in the repo (only the one-line summary at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives); the two design buckets were therefore RE-DERIVED as (A) no neutral capability model — the GitHub dossier §4 names eleven flags the port needs while `probeCapabilities` reads three project fields and `capabilityGap` is computed in GitLab terms — and (B) no port-level transport/auth policy — GitHub thread resolution is GraphQL-only and needs PAT-vs-App auth, while bounded reads/pagination caps (AUD-S10) and idempotent-GET retry/deadlines (AUD-S11) live inside the GitLab client. Both are recorded as a re-derivation, not as a citation. **OPEN SUB-QUESTION (operator)**: whether the Actions entrypoint (E10-S16) stays in scope — `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", but the action is packaging on top of an adapter and is the one story whose absence leaves everything else useful; default is to keep it last and independently droppable. **Consequence to watch (E10-S04)**: forcing every capability to be stated explicitly may turn a GitLab arming path that passes today into an honest capability gap — a user-visible behaviour change that gets its OWN decision row and changelog entry, never absorbed silently into "E10 refactor". Revert: re-lock E10 in `later-phases.md`, delete the spec directory; nothing here is published API (`forge.RunPort` is `internal/`), so no compatibility window applies. | -| D-141 | 2026-08-10 | **E11 (Rego complex-rule backend) implementation is UNLOCKED by direct operator instruction; `openspec/specs/p5-e11-rego-backend/spec.md` decomposes it into 13 stories under ADR-0002 v2.** E11's CONTRACT was already unlocked by D-017; what was gated was IMPLEMENTATION, twice: "after Phase 4" (satisfied — the Phase-4 adoption gate closed with D-042) and, per D-017, **evidence-based per rule** ("each ported rule tries CEL first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling"). This row records the operator lifting that per-rule evidence gate. **What it does NOT waive**: the DESIGN need the gate was protecting — E11-S01 still requires a written tier-1 ceiling document with concrete rules per shape (multi-pass, cross-manifest, set-difference, graph-relationship), and any shape found CEL-expressible is struck from scope. **What it does NOT authorize**: WASM or gRPC predicate backends (still Locked under D-012 — this unlocks Rego only); domain-aware joins and in-process Go rule plugins (D-017 DECLINED both permanently — not deferred); giving Rego any control over aggregation, effects, or points (ADR-0002 v2 boundary); any `EvaluationInput` change. **Two constraints found during design that shape the epic**: (1) **E11 is the first epic whose DoD is `git diff schemas/` != 0.** P3-E1-S02's backend-neutrality guarantee ("no field naming a predicate backend anywhere in the schema") applies to `EvaluationInput` and HOLDS — no decision contract changes — but `schemas/policy/v1alpha1/merge-policy.schema.json` defines the predicate leaf as `additionalProperties:false, required:["cel"]`, so a `rego:` leaf IS a policy-schema change. `API_STABILITY.md:19` permits exactly this within `v1alpha1` as an announced additive change with an openspec change and no `apiVersion` bump; the change is backward-compatible and deliberately forward-INcompatible (an older binary rejects a `rego:` leaf by strict-decode, which is the correct direction — it must not silently ignore a rule it cannot evaluate). A reviewer applying the previous epics' `git diff schemas/ == 0` habit will flag the correct change as a violation; E11-S02-04 scopes the drift guard rather than deleting it. (2) **The reflexive safety measure violates AGENTS.md rule 7.** Rego ships `time.now_ns()`, `rand.intn()`, and `http.send()`, all of which are denied structurally by the D-013 capability sandbox (E11-S04, compile-time failure, golden allowlist so an OPA upgrade cannot widen it unnoticed) — but bounding evaluation with a WALL-CLOCK TIMEOUT would itself be a rule-7 violation, making the same policy over the same ChangeSet decide differently on a slow runner. E11-S06 therefore requires a machine-independent evaluation budget, and exceeding it must be a PROCESS ERROR that can never be a policy outcome; "timeout → BLOCK" is explicitly rejected as machine-dependent while merely LOOKING fail-closed. **Also fixed by the spec**: zero violations NEVER proves a required obligation (`later-phases.md`'s explicit polarity rule; E11-S07-02 tests the failing polarity). **OPEN SUB-QUESTION (operator)**: `github.com/open-policy-agent/opa` is a large dependency with a large transitive tree on a project shipping cosign/SLSA provenance, `govulncheck`, and Scorecard — recommended default is accept-and-pin (a hand-rolled evaluator would be far worse), but it materially changes binary size and vulnerability surface and is flagged for explicit ack. Revert: re-assert the D-017 per-rule evidence gate, delete the spec directory, leave the `# locked: D-012` quarantine marker in place. | +| D-141 | 2026-08-10 | **E11 (Rego complex-rule backend) implementation is UNLOCKED by direct operator instruction; `openspec/specs/p5-e11-rego-backend/spec.md` decomposes it into 13 stories under ADR-0002 v2.** E11's CONTRACT was already unlocked by D-017; what was gated was IMPLEMENTATION, twice: "after Phase 4" (satisfied — the Phase-4 adoption gate closed with D-042) and, per D-017, **evidence-based per rule** ("each ported rule tries CEL first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling"). This row records the operator lifting that per-rule evidence gate. **What it does NOT waive**: the DESIGN need the gate was protecting — E11-S01 still requires a written tier-1 ceiling document with concrete rules per shape (multi-pass, cross-manifest, set-difference, graph-relationship), and any shape found CEL-expressible is struck from scope. **What it does NOT authorize**: WASM or gRPC predicate backends (still Locked under D-012 — this unlocks Rego only); domain-aware joins and in-process Go rule plugins (D-017 DECLINED both permanently — not deferred); giving Rego any control over aggregation, effects, or points (ADR-0002 v2 boundary); any `EvaluationInput` change. **Two constraints found during design that shape the epic**: (1) **E11 is the first epic whose DoD is `git diff schemas/` != 0.** P3-E1-S02's backend-neutrality guarantee ("no field naming a predicate backend anywhere in the schema") applies to `EvaluationInput` and HOLDS — no decision contract changes — but `schemas/policy/v1alpha1/merge-policy.schema.json` defines the predicate leaf as `additionalProperties:false, required:["cel"]`, so a `rego:` leaf IS a policy-schema change. `API_STABILITY.md:19` permits exactly this within `v1alpha1` as an announced additive change with an openspec change and no `apiVersion` bump; the change is backward-compatible and deliberately forward-INcompatible (an older binary rejects a `rego:` leaf by strict-decode, which is the correct direction — it must not silently ignore a rule it cannot evaluate). A reviewer applying the previous epics' `git diff schemas/ == 0` habit will flag the correct change as a violation; E11-S02-04 scopes the drift guard rather than deleting it. (2) **The reflexive safety measure violates AGENTS.md rule 7.** Rego ships `time.now_ns()`, `rand.intn()`, and `http.send()`, all of which are denied structurally by the D-013 capability sandbox (E11-S04, compile-time failure, golden allowlist so an OPA upgrade cannot widen it unnoticed) — but bounding evaluation with a WALL-CLOCK TIMEOUT would itself be a rule-7 violation, making the same policy over the same ChangeSet decide differently on a slow runner. E11-S06 therefore requires a machine-independent evaluation budget, and exceeding it must be a PROCESS ERROR that can never be a policy outcome; "timeout → BLOCK" is explicitly rejected as machine-dependent while merely LOOKING fail-closed. **Also fixed by the spec**: zero violations NEVER proves a required obligation (`later-phases.md`'s explicit polarity rule; E11-S07-02 tests the failing polarity). **🔴 BLOCKING OPEN SUB-QUESTION (operator) — adopting OPA narrows rule 7's MECHANISM, and both existing purity gates would miss it.** Verified during the design session: `internal/core/purity_test.go` flags only each guarded file's OWN imports (`math/rand`, `crypto/rand`, `net`, `net/*`) and selectors (`os.Getenv`, `time.Now`), and `.golangci.yml`'s `pure-tree` depguard is `list-mode: lax`, deny-only, over DIRECT imports — **neither is transitive**. A file in `internal/core/**` importing `github.com/open-policy-agent/opa/rego` therefore passes both gates GREEN while transitively linking `net/http` (OPA ships the `http.send` builtin), defeating the `net` deny that encodes D-123 / rule 7 invisibly. S04's capability sandbox makes `http.send` uncallable FROM POLICY — the real threat — but the guarantee's nature changes from "the network stack is not linked into the decision path" (structural, greppable) to "linked but unreachable from policy" (behavioural, resting on a capability file). That is a hard-rule change and cannot be made by a story. Options: **(d1)** accept the narrowing with an ADR-0011/rule-7 amendment plus a transitive `go list -deps` purity check allowlisting exactly the OPA path (RECOMMENDED); **(d2)** keep the guarded tree OPA-free by injecting an evaluator from `cmd/assent` — honest, but it moves part of the decision path outside the tree rule 7 guards; **(d3)** drop OPA (a hand-rolled evaluator would be far worse; rejected unless d1 and d2 are). **E11-S04 is blocked on this answer** (it decides the evaluator's package and its gate); S01–S03 are unblocked. Separately on supply chain: OPA is a large dependency with a large transitive tree on a project shipping cosign/SLSA provenance, `govulncheck`, and Scorecard — recommended default is accept-and-pin, with S03 recording the binary-size delta. Revert: re-assert the D-017 per-rule evidence gate, delete the spec directory, leave the `# locked: D-012` quarantine marker in place. | diff --git a/openspec/specs/backlog.md b/openspec/specs/backlog.md index 422972b2..3236de3e 100644 --- a/openspec/specs/backlog.md +++ b/openspec/specs/backlog.md @@ -546,15 +546,22 @@ table. Every one of the **37** 2026-08-06 finding IDs is dispositioned in Spec: [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md) · ADR: **0021** (the seam) · Dossier: [forge-dossier-github.md](../../docs/planning/forge-dossier-github.md). -**Ordering is normative — the seam (S01–S05) lands before the first GitHub API call**, because -the conformance suite is unimportable today and an adapter written now would have no executable -contract to satisfy. S02/S04 are core-contract and require **maintainer LGTM** (GOVERNANCE); -`/agent-loop-auto` must surface them rather than auto-merge. +**Ordering is normative — S00 before any code, and the seam (S01–S05) before the first GitHub +API call.** An adversarial review of the first draft (2026-08-10) found **two P0 representation +defects** by reading the port against the code: the port addresses head content by branch name +in one project, so **every GitHub fork PR would mint a fabricated whole-file DELETE** +(`run.go:274` → `fileAtRefOrAbsent` → `OneSidedLifecycle`); and `$defs.pins` is +`additionalProperties:false` with a **single-string** `capabilityGap` required iff +`mergeResultDigest` is null, so an eleven-capability report **has nowhere valid to be +recorded**. Both are decided in ADR-0021 (items 5–8) and gated by S00. S00/S02/S04 are +core-contract and require **maintainer LGTM** (GOVERNANCE); `/agent-loop-auto` must surface +them rather than auto-merge. | ID | Story | Execution | Depends on | Gate contribution | | --- | --- | --- | --- | --- | -| E10-S01 | Extract the conformance suite into an importable package (`RunSuite`) | **[autonomous]** | none | **story zero** — without it no adapter can be TDD'd | -| E10-S02 | ⚠️ `forge.RunPort` named composite port + depguard denies both adapters from `cmd` | **[autonomous · engine-grade · LGTM]** | S01 | one neutral seam; ARCH-02 cannot recur | +| E10-S00 | ⚠️ GitHub addressing & representation model (4 questions, ~1 page) | **[autonomous · design · LGTM]** | none | **do first** — kills both P0s before the port freezes | +| E10-S01 | Extract the conformance suite into an importable package + observation surface | **[autonomous]** | S00 | first **code** story; no assertion may be weakened | +| E10-S02 | ⚠️ `forge.RunPort` + **neutral factory** + MR-relative addressing + identity | **[autonomous · engine-grade · LGTM]** | S00, S01 | one neutral seam; ARCH-02 cannot recur | | E10-S03 | ⚠️ Collapse `SyntheticDigest` onto `Snapshot.Heads.MergeResultDigest` | **[autonomous · engine-grade]** | S02 | digest scheme adapter-owned; allowlist emptied | | E10-S04 | ⚠️ Neutral capability model — `unknown` never arms | **[autonomous · engine-grade · LGTM]** | S02 | one fail-closed guarantee, not two | | E10-S05 | Port-level transport requirements (bounded reads, caps, GET-only retry, deadlines) | **[autonomous]** | S01, S04 | availability behaviour can't diverge per adapter | @@ -564,9 +571,9 @@ contract to satisfy. S02/S04 are core-contract and require **maintainer LGTM** ( | E10-S09 | ⚠️ GitHub capability report (11 flags; unverified ⇒ `unknown`) | **[autonomous · engine-grade]** | S04, S06 | honest gaps; exhaustiveness enforced | | E10-S10 | ⚠️ GitHub Reconcile writes (ADR-0019 parity, GraphQL thread resolution) | **[autonomous · engine-grade]** | S07–S09 | same engine, second adapter | | E10-S11 | ⚠️ SHA-guarded merge + deferred arming + revoke-on-push | **[autonomous · engine-grade]** | S10 | ADR-0015 §2 on GitHub | -| E10-S12 | ⚠️ Capability gaps fail closed (3 deltas, `merges == 0` proven) | **[autonomous · engine-grade]** | S11 | the polarity reviews keep finding untested | +| E10-S12 | ⚠️ Capability gaps fail closed — `merges == 0` **and paired `merges == 1`** | **[autonomous · engine-grade]** | S11 | positive control mandatory; else vacuous | | E10-S13 | Forge selection in `run`/`doctor`; ambiguity fails closed | **[autonomous]** | S12 | no default-to-GitLab | -| E10-S14 | Conformance parity + `github-deferred` catalog flip (D-084 dispositioned) | **[autonomous]** | S13 | no bare deferrals; both factories in CI | +| E10-S14 | Conformance parity — **every** row needs an adapter disposition, not just deferrals | **[autonomous]** | S13 | else GitHub ships with 0 trust-boundary cases proven | | E10-S15 | Docs & maturity truth (README tier, C4, `--forge`, dossier items) | **[autonomous]** | S14 | no doc claims an `unknown` capability | | E10-S16 | Actions entrypoint (`action.yml`, pinned binary, base-ref trust) | **[autonomous — scope-flagged]** | S15 | **D-140 open sub-question**; independently droppable | | E10-S17 | Exit gate | **[autonomous]** | S01–S16 | **the E10 exit gate** | @@ -584,9 +591,10 @@ LGTM** (published contract + the decision path itself). Independent of E10; may | ID | Story | Execution | Depends on | Gate contribution | | --- | --- | --- | --- | --- | | E11-S01 | Record the tier-1 (CEL) ceiling with concrete exceeding rules | **[autonomous]** | none | **do first** — a CEL-expressible shape is struck from scope | -| E11-S02 | ⚠️ Additive `rego:` leaf in the policy schema (announced, no `apiVersion` bump) | **[autonomous · engine-grade · LGTM]** | S01 | drift guard scoped, not deleted | +| E11-S06′ | ⚠️ **SPIKE first**: does OPA expose a deterministic (non-wall-clock) eval budget? | **[autonomous · spike]** | S01 | if not, S06 stalls the epic *after* S02+S03 commit | +| E11-S02 | ⚠️ Additive `rego:` leaf in the policy schema (announced, no `apiVersion` bump) | **[autonomous · engine-grade · LGTM]** | S01, S06′ | drift guard scoped; both polarities tested | | E11-S03 | Module loading from the **target ref**; compile failure is a lint hard error | **[autonomous]** | S02 | no second, laxer load path | -| E11-S04 | ⚠️ OPA capability sandbox (D-013) — denied builtins fail at **compile** | **[autonomous · engine-grade · LGTM]** | S03 | rule 7 structurally; golden allowlist | +| E11-S04 | 🔴 OPA capability sandbox — **blocked on the operator's rule-7 answer (d1/d2/d3)** | **[autonomous · engine-grade · LGTM]** | S03 + operator | both purity gates are non-transitive; see D-141 | | E11-S05 | ⚠️ Input binding to the identical `EvaluationInput` | **[autonomous · engine-grade]** | S04 | proves P3-E1-S02 neutrality empirically | | E11-S06 | ⚠️ Deterministic evaluation budget (never wall-clock) | **[autonomous · engine-grade · LGTM]** | S05 | N≥100 identical runs; budget ≠ decision | | E11-S07 | ⚠️ Violations → findings; **zero violations never proves an obligation** | **[autonomous · engine-grade · LGTM]** | S06 | the failing polarity is tested | diff --git a/openspec/specs/p5-e10-github-forge/spec.md b/openspec/specs/p5-e10-github-forge/spec.md index 84c9f150..8df41f05 100644 --- a/openspec/specs/p5-e10-github-forge/spec.md +++ b/openspec/specs/p5-e10-github-forge/spec.md @@ -12,13 +12,30 @@ half-built. AUD-S15 lifted `MRInfo`/`ErrNotFound` into `internal/forge/port.go`, `cmd/assent`'s `forgePort` is still an anonymous interface literal at the call site, `run.go` still calls `gitlab.SyntheticDigest`, capability vocabulary is GitLab-private, transport policy (bounded reads, pagination caps, retry, deadlines) lives inside -`internal/forge/gitlab`, and **all ~1,166 lines of `internal/forge/conformance` are in +`internal/forge/gitlab`, and **all 1,166 lines of `internal/forge/conformance` are in `_test.go` files Go cannot import** — so the suite that defines "behaves like a forge" cannot -be run by a second adapter. `catalog.yaml`'s `github-deferred` rows (D-084) are therefore -unflippable by construction, not merely unimplemented. The 2026-08-09 audit recorded this as -ARCH-18/ARCH-19: `e10-forge-port-lift.md` under-scopes the epic by two design buckets -(capability model; transport/auth policy) plus the unimportable suite. ADR-0021 decides all -three; this epic executes it. +be run by a second adapter. The 2026-08-09 audit recorded this as ARCH-18/ARCH-19: +`e10-forge-port-lift.md` under-scopes the epic by two design buckets (capability model; +transport/auth policy) plus the unimportable suite. + +**An adversarial review of this spec's first draft (2026-08-10) found three further buckets, +two of them P0, and they are why S00 exists.** All are *addressing or representation* +failures — a class the P1-E3-S03 dossier structurally could not surface, because it studied +GitHub's **endpoints**, not how the port **names** things: fork-head addressing (the port +reads head content by branch name in one project, so every GitHub fork PR would mint a +fabricated whole-file DELETE); HTTP-status → sentinel collapse (GitHub 404s permission +denials, so *forbidden* would read as *absent*); and the record surface (`pins` is +`additionalProperties:false` with a **single-string** `capabilityGap` required *iff* +`mergeResultDigest` is null — an eleven-capability report has nowhere valid to live). +**ADR-0021 items 5–8 decide all three**; this epic executes them. + +*Corrections carried from that review, kept rather than quietly edited away:* an earlier +draft claimed the extraction would unblock `catalog.yaml`'s `github-deferred` rows. **False** — +both rows are `level: L3, package: test/e2e`, gated on live GitHub infrastructure (S18), not +on importability. The extraction's real justification is the executable contract. A second +draft claimed `capabilityGap` "already models the absent-capability case". **Also false** — it +models exactly one capability, merge-result pinning, which is why it is singular and coupled +to that field. **Key ground truth (de-risks the epic):** - **The dossier is the spec input, not guesswork.** `docs/planning/forge-dossier-github.md` @@ -33,15 +50,18 @@ three; this epic executes it. *packaging* is wrong), P3-E5 fixtures under `docs/contracts/p3-e5-publication-protocol/fixtures/`, and the E4 GitLab adapter as the reference implementation of every port method. -- **Frozen schemas stay frozen:** epic DoD is **`git diff schemas/` == 0**. GitHub introduces - no new decision-contract field; `capabilityGap` and `pins.mergeResultDigest` already model - the absent-capability case (ADR-0017 §1, nullable only when the capability is absent). +- **Frozen schemas stay frozen — at a stated cost.** Epic DoD is **`git diff schemas/` == 0**, + which is only achievable because ADR-0021 item 8 scopes the multi-capability report to + `doctor` output and arming-refusal reasons, **never the DecisionRecord**. The cost is + explicit: a capability gap that blocks a merge leaves no trace in the record beyond the + existing single `capabilityGap` string. Widening `pins` is a `v1alpha2` conversation. - **`internal/core` stays I/O-free** (`TestCorePurity`); everything here is `internal/forge/**` plus the `cmd/assent` edge. - **The seam is `internal/`, not public API.** `forge.RunPort` carries no `apiVersion` and no compatibility window; getting it wrong costs a refactor inside this epic, never a break. -**Scope**: **Seam wave** — (S01) conformance suite extracted to an importable package; (S02) +**Scope**: **Seam wave** — (S00) the GitHub addressing & representation model, written before +any code; (S01) conformance suite extracted to an importable package; (S02) `forge.RunPort` named composite port + depguard; (S03) `SyntheticDigest` collapse; (S04) neutral capability model; (S05) port-level transport requirements. **Adapter wave** — (S06) GitHub client (REST + GraphQL, PAT + App auth); (S07) Snapshot; (S08) Resolve → @@ -76,9 +96,13 @@ capability model core-contract work per GOVERNANCE, and `/agent-loop-auto`'s sto already require surfacing public-API/core-contract changes rather than auto-merging them. S18 **`[infra-gated · operator]`** (a real GitHub repository, live PRs, a token). -**Dependency order**: S01 → S02 → {S03, S04} → S05 → S06 → {S07, S08, S09} → S10 → S11 → S12 -→ S13 → S14 → {S15, S16} → S17 → S18. **Do first: S01** — until the suite is importable, every -adapter story is written against no executable contract. +**Dependency order**: **S00** → S01 → S02 → {S03, S04} → S05 → S06 → {S07, S08, S09} → S10 → +S11 → S12 → S13 → S14 → {S15, S16} → S17 → S18. **Do first: S00** — the adversarial review +found two P0 representation defects by reading the port against the code, and both would +otherwise surface at S07 with the port signature frozen and the golden corpus pinned. S00 is +four questions and roughly a page; it is the cheapest risk reduction in the epic. **S01 is the +first code story** — until the suite is importable, every adapter story is written against no +executable contract. ## Judgment calls (decide-and-log / operator) @@ -124,6 +148,54 @@ runs it. ## Seam wave +### E10-S00 — GitHub addressing & representation model `[autonomous · design · maintainer LGTM]` + +- **Goal**: answer, on paper and before any code, the four questions whose wrong answers the + adversarial review demonstrated are P0 — each of which would otherwise be discovered after + the port signature is frozen. +- **Dependencies**: none. **This is the first story in the epic.** +- **Definition of done**: `docs/planning/github-addressing-model.md` answers all four with a + decision, and each answer names the conformance case that will prove it. Answers that + contradict ADR-0021 items 5–8 amend the ADR in the same change. + +- **REQ-E10-S00-01** — **How does the port name the head content of a fork PR?** Given + `run.go:270,274` reads base/head by branch name in one project and `MRInfo` carries no + source-repository identifier, when the model is written, then it fixes the addressing shape + (ADR-0021 item 5 proposes `FileAtBase(mr, path)` / `FileAtHead(mr, path)`), and states the + concrete failure it prevents: a fork PR 404s → `fileAtRefOrAbsent` → `nil` → + `OneSidedLifecycle` → **fabricated whole-file DELETE**. + - Test: `docs/planning/github-addressing-model.md` + - Verify: manual review; the named conformance case appears in S01's catalog + - Level: L0 +- **REQ-E10-S00-02** — **What operationally decidable predicate makes each of the eleven + capability flags `supported`, per forge?** Given a tri-state with no membership test is a + vocabulary rather than a model, when the model is written, then every flag has a concrete + probeable condition per adapter — **explicitly including `protected-pipeline-source`** + (ADR-0015 §4's arming prerequisite, which has no single readable GitHub analogue) and + **`eligible-approval-evidence`** (dossier §2: no API returns the computed per-PR eligible + code owners). Where no predicate exists, the model says so and the consequence — the gate + cannot be armed on that forge — is stated as a product limitation, never papered over with a + heuristic (the SEC-04 failure mode). + - Test: `docs/planning/github-addressing-model.md` + - Verify: manual review; S04's enum cannot be frozen until every flag has an entry + - Level: L0 +- **REQ-E10-S00-03** — **Where does an eleven-capability report live in the record?** Given + `$defs.pins` is `additionalProperties:false` with a single-string `capabilityGap` required + *iff* `mergeResultDigest` is null, when the model is written, then it confirms or overturns + ADR-0021 item 8's choice (report scoped to `doctor`/refusal reasons, not the DecisionRecord) + and states the audit-trail cost plainly. Recording it in the record's open top-level object + is rejected — an unvalidated safety-bearing field is a fail-closed guarantee in name only. + - Test: `docs/planning/github-addressing-model.md` + - Verify: manual review + - Level: L0 +- **REQ-E10-S00-04** — **What is each adapter's HTTP-status → port-sentinel mapping?** Given + `forge.ErrNotFound` is a semantic *presence* signal and GitHub 404s permission denials, when + the model is written, then it fixes the mapping per adapter and names the conformance case + proving *forbidden* never renders as *absent*. + - Test: `docs/planning/github-addressing-model.md` + - Verify: manual review; the named case appears in S01's catalog + - Level: L0 + ### E10-S01 — Extract the conformance suite into an importable package `[autonomous]` - **Goal**: a second adapter can execute the *existing* forge conformance cases without @@ -131,10 +203,16 @@ runs it. - **Why now**: `internal/forge/conformance` is 1,166 lines across four `_test.go` files. Go cannot import `_test.go`, so today the only way to conformance-test a new adapter is duplication — which guarantees drift and makes D-084's `github-deferred` rows unflippable. -- **Dependencies**: none. **This is story zero.** +- **Dependencies**: S00. - **Definition of done**: case bodies live in importable Go; `go test ./internal/forge/...` passes with **no case deleted, renamed, or weakened**; the GitLab entry point is a thin `_test.go` calling the shared runner. +- **The tension to resolve deliberately, not cheaply**: the existing cases assert on + `*fake.Forge` internals — `sha_guard_test.go:49` takes `*fake.Forge`, and + `reconciliation_test.go:220` type-asserts to it — reading recorded writes (`Merges`, + `Approvals`). A `Factory` returning a `forge.RunPort` is therefore only half a contract. The + cheap resolution is to weaken assertions to what both backends can observe, which is exactly + how a suite silently stops proving the SHA-guard. REQ-E10-S01-04 forbids that resolution. - **REQ-E10-S01-01** — Given the conformance cases currently in `_test.go`, when the package is restructured, then it exports `conformance.RunSuite(t *testing.T, f Factory)` where @@ -156,15 +234,33 @@ runs it. - Test: `internal/forge/conformance/catalog.yaml`, `catalog_test.go` - Verify: `go test ./internal/forge/conformance/ -run TestCatalogStrictDecode` - Level: L1 +- **REQ-E10-S01-04** — Given the cases assert on `*fake.Forge` internals, when the suite is + extracted, then the package defines an explicit **port-level observation surface** (the + recorded-writes view a case may assert on: merges, approvals, threads, comments) that both + adapters implement, and **no assertion is downgraded to accommodate a backend**. Proven by a + test asserting the extracted SHA-guard case still observes a merge *attempt count*, not + merely a returned error — the specific weakening this REQ exists to prevent. + - Test: `internal/forge/conformance/observe.go`, `suite_test.go` + - Verify: `go test ./internal/forge/conformance/ -run TestSHAGuardObservesMergeAttempts` + - Level: L1 ### E10-S02 — `forge.RunPort` composite port + depguard `[autonomous · engine-grade · maintainer LGTM]` - **Goal**: `cmd/assent` depends on one named, forge-neutral interface and on no concrete adapter package. -- **Dependencies**: S01 (so the port change is proven by an executable suite). +- **Dependencies**: S00, S01 (so the port change is proven by an executable suite). - **Definition of done**: `forge.RunPort` declared in `internal/forge`; `cmd/assent`'s - anonymous port literal deleted; depguard denies **both** concrete adapters from `cmd/assent`; - zero behaviour change (goldens and conformance byte-identical). + anonymous port literal deleted; **the neutral adapter factory lands in this story**; + depguard denies **both** concrete adapters from `cmd/assent`; zero behaviour change + (goldens and conformance byte-identical). +- **Corrected after adversarial review — the first draft of this story could not close.** It + required depguard to deny both adapters "with no symbol allowlist", but `cmd/assent/main.go:72,83` + calls `gitlab.New(endpoint, token, botAuthor)` and no story supplied a neutral factory until + **S13**, ten stories later. Worse, `hack/lint/depguard_test.sh:356-363` is an anti-vacuity + guard that **fails the build** unless it sees ≥4 `gitlab.` references in + `cmd/assent` *including* `New` and `SyntheticDigest` — and `task lint` is this story's own + Verify command. The factory therefore moves from S13 into S02, and REQ-E10-S02-04 replaces + the scanner's positive control rather than deleting it. - **REQ-E10-S02-01** — Given ADR-0021 §1, when `forge.RunPort` is declared, then it composes `forge.Forge`, `forge.Snapshotter`, `forge.Resolver`, `Describe(project, mr string) @@ -186,6 +282,31 @@ runs it. - Test: `internal/forge/fake/fake.go`, `internal/forge/conformance/suite_test.go` - Verify: `go test ./internal/forge/...` - Level: L1 +- **REQ-E10-S02-04** — Given `hack/lint/depguard_test.sh:356-363` hard-fails unless it sees + `gitlab.New` and `gitlab.SyntheticDigest` in `cmd/assent`, when those call-sites are removed + by the factory, then the scanner's positive control is **replaced, never deleted**: it + asserts against a violating *copy* of `cmd/assent` (the polarity-A tree the script already + builds in `$WORK`), so the scanner is still proven to see real code while the real tree + legitimately contains zero adapter symbols. + - Test: `hack/lint/depguard_test.sh` + - Verify: `task lint` + - Level: L1 +- **REQ-E10-S02-05** — Given ADR-0021 item 5, when `RunPort` is declared, then content is + addressed **relative to the merge request** (`FileAtBase(mr, path)` / `FileAtHead(mr, + path)`), not by `(project, branch-name)`, so an adapter owns how it reaches a fork's head. + A conformance case proves a **fork MR with an unchanged governed file yields NO lifecycle + event** on every adapter — the fabricated-DELETE defect. Smuggling `refs/pull/N/head` into + `MRInfo.SourceBranch` is rejected: it corrupts a documented field and leaks into rendering. + - Test: `internal/forge/port.go`, `internal/forge/conformance/`, `cmd/assent/run.go` + - Verify: `go test ./... -run TestForkMRNoFabricatedDelete` + - Level: L1 +- **REQ-E10-S02-06** — Given ADR-0021 item 7, when the port is declared, then it exposes the + **authenticated identity**, and a case proves markers are recognised as our own under + **both** auth shapes — a PAT identity is a `User`, so an "exclude any bot" filter would make + assent blind to its own comments and duplicate threads forever. + - Test: `internal/forge/port.go`, `internal/forge/conformance/` + - Verify: `go test ./internal/forge/... -run TestOwnMarkersRecognisedBothAuthShapes` + - Level: L1 ### E10-S03 — Collapse `SyntheticDigest` onto `Snapshot.Heads.MergeResultDigest` `[autonomous · engine-grade]` @@ -230,9 +351,21 @@ runs it. - **REQ-E10-S04-02** — Given ADR-0021 §3 and judgment call (d), when a capability is reported `unknown`, then every arming decision treats it as `absent` — proven by a table test over all three states asserting `unknown` and `absent` produce the identical non-arming outcome - and a distinguishable *reason* string. + and a distinguishable *reason* string. **Paired positive control (required, not optional):** + the same table asserts that all-capabilities-`supported` **does** arm. Without it the REQ is + satisfied by an adapter that never arms under any conditions — the documented + tests-that-cannot-fail defect class, whose in-repo antidote is the mutation-control pattern + at `hack/lint/depguard_test.sh:356-363`. - Test: `internal/forge/capability_test.go` - - Verify: `go test ./internal/forge/ -run TestUnknownDoesNotArm` + - Verify: `go test ./internal/forge/ -run 'TestUnknownDoesNotArm|TestAllSupportedDoesArm'` + - Level: L1 +- **REQ-E10-S04-05** — Given a transport failure during probing is not the same fact as an + absent capability, when a probe request fails (5xx, timeout), then it is a **hard process + error**, never a silent downgrade to `unknown`. Otherwise a 502 on one endpoint flips + APPROVE→REVIEW on an otherwise successful exit-0 run, making decisions network-dependent and + breaking ReplayBundle reproducibility — the same separation E11's evaluation budget gets right. + - Test: `internal/forge/capability.go`, `capability_test.go` + - Verify: `go test ./internal/forge/ -run TestProbeFailureIsNotUnknown` - Level: L1 - **REQ-E10-S04-03** — Given `capabilityGap` is port-computed, when the GitLab adapter is migrated to return a `CapabilityReport`, then no adapter computes a gap itself, and any @@ -390,8 +523,12 @@ runs it. - Verify: `go test ./internal/forge/... -run TestConformance` - Level: L1 - **REQ-E10-S10-02** — Given contributor marker spoofing (E4-S09 precedent), when threads are - listed, then the author-identity filter excludes non-bot authors, and a malformed marker is - **skipped with a warning** rather than bricking reconciliation (RELI-06 precedent). + listed, then the filter matches **the token identity's own user/app id** (`GET /user` / + `GET /app`) — the GitLab precedent is `first.Author.Username != c.botAuthor`, our *specific* + identity, and dossier C9 says "identical to GitLab: match `user.id` of the token identity". + An "exclude any bot" filter is explicitly wrong: this repo runs Renovate, so any second app + causes marker collision, and anyone who can install an app gets marker spoofing. A malformed + marker is **skipped with a warning** rather than bricking reconciliation (RELI-06 precedent). - Test: `internal/forge/github/reconcile_test.go` - Verify: `go test ./internal/forge/github/ -run TestMarkerSpoofAndMalformed` - Level: L2 @@ -415,6 +552,24 @@ runs it. - Test: `internal/forge/github/merge.go`, `merge_test.go` - Verify: `go test ./internal/forge/github/ -run TestArmingRevokeOnPush` - Level: L2 +- **REQ-E10-S11-03** — Given ADR-0017 §1 requires `pins.mergeResultDigest` to be the + **evaluated** merge-result digest, and given a merge queue builds + `gh-readonly-queue/{base}/…` containing the target **plus other queued PRs** (dossier C14), + when the queue is in use, then the adapter proves at the **hermetic** tier that the recorded + pin corresponds to the commit the queue actually merges — or reports + `merge-result-pinning` as a capability gap and does not arm. The only catalog row that would + otherwise catch this (`github-merge-queue-sha-guard`) is `level: L3, package: test/e2e`, so + nothing in the autonomous slice can detect the divergence. + - Test: `internal/forge/github/merge_test.go`, `internal/forge/conformance/` + - Verify: `go test ./internal/forge/github/ -run TestMergeQueuePinMatchesMergedCommit` + - Level: L2 +- **REQ-E10-S11-04** — Given "Require merge queue" may reject `PUT /pulls/{n}/merge` outright, + when the queue is enabled, then S00's predicate table records whether SHA-pinned direct merge + and queue-based merge are **mutually exclusive paths** rather than one, and the adapter + implements whichever the forge permits — failing closed if neither can be SHA-pinned. + - Test: `docs/planning/github-addressing-model.md`, `internal/forge/github/merge.go` + - Verify: `go test ./internal/forge/github/ -run TestQueueAndDirectMergeExclusivity` + - Level: L2 ### E10-S12 — Capability gaps fail closed on GitHub `[autonomous · engine-grade]` @@ -431,10 +586,23 @@ runs it. - Level: L1 - **REQ-E10-S12-02** — Given fail-closed claims are worthless untested, when each of the three deltas is simulated absent, then a table test asserts `merges == 0` for all three — the - polarity E6/AUD reviews repeatedly found untested. + polarity E6/AUD reviews repeatedly found untested — **and, in the same table, that the + all-capabilities-present case yields `merges == 1`.** The positive control is mandatory: + without it every row passes vacuously on an adapter that never arms at all, and S17's exit + gate goes green on an adapter that does nothing. - Test: `internal/forge/github/failclosed_test.go` - Verify: `go test ./internal/forge/github/ -run TestDeltasFailClosed` - Level: L1 +- **REQ-E10-S12-03** — Given the adversarial review established that a GitHub adapter which + **never arms** is a plausible shipped outcome (`protected-pipeline-source` and + `eligible-approval-evidence` both plausibly `unknown` forever), when this story closes, then + the epic records which capabilities are actually `supported` against a real repository, and + **if no arming path is reachable, that is surfaced to the operator as a product limitation + before S15 writes the maturity table** — never discovered at S18 when the live proof turns + out to be unreachable. + - Test: `docs/planning/github-addressing-model.md` (S00's predicate table, filled in) + - Verify: manual — S15 is blocked until the table has real values + - Level: L0 ## Integration wave @@ -451,9 +619,9 @@ runs it. - Test: `cmd/assent/forge_select.go`, `forge_select_test.go` - Verify: `go test ./cmd/... -run TestForgeSelection` - Level: L1 -- **REQ-E10-S13-02** — Given S02, when forge selection is wired, then construction happens - behind a factory returning `forge.RunPort`, and `task lint`'s depguard still denies - `cmd/assent` importing either adapter package. +- **REQ-E10-S13-02** — Given the factory now lands in **S02** (not here — see that story's + correction note), when forge selection is wired, then it selects *through* the existing + factory and `task lint`'s depguard still denies `cmd/assent` importing either adapter package. - Test: `hack/lint/depguard_test.sh` - Verify: `task lint` - Level: L1 @@ -465,11 +633,19 @@ runs it. `github-deferred` row in `catalog.yaml` is either flipped to `both` or **retains the deferral with a named, cited reason**; D-084 is dispositioned. -- **REQ-E10-S14-01** — Given D-084, when the catalog is updated, then no row remains - `github-deferred` without a reason field naming the blocking open-verification item, and a - test fails on any bare deferral. +- **REQ-E10-S14-01** — Given **every one of the 15 non-deferred catalog rows is + `forge: gitlab`**, when the catalog is updated, then **every row** — not only + `github-deferred` ones — carries an explicit per-adapter disposition (`both`, or a single + forge **plus a cited reason**), and a test fails on any bare `forge: gitlab` row. + *Corrected after adversarial review*: the first draft required reasons only for + `github-deferred` rows, which would have shipped the GitHub adapter with **zero** of the + trust-boundary cases proven — `TestRunForkContextAdvisoryOnly`, + `TestRunPolicyFromTargetRefOnly`, `TestDoctorForgeInsecureCITopology`, + `TestConformanceSpoofedMarkerIgnored`, `TestRunEnumerationIncompleteNeverApproves` + (`exitgate_test.go:27-40`) — while S17 went green. Those are precisely the cases most likely + to differ between forges (`pull_request_target`, `GITHUB_TOKEN` scope, `refs/pull/N/*`). - Test: `internal/forge/conformance/catalog.yaml`, `catalog_test.go` - - Verify: `go test ./internal/forge/conformance/ -run TestNoBareDeferrals` + - Verify: `go test ./internal/forge/conformance/ -run TestEveryRowHasAdapterDisposition` - Level: L1 - **REQ-E10-S14-02** — Given both adapters implement `forge.RunPort`, when CI runs, then `RunSuite` executes against **both** factories in the same job, and a new case added for one diff --git a/openspec/specs/p5-e11-rego-backend/spec.md b/openspec/specs/p5-e11-rego-backend/spec.md index f3c37f20..93244270 100644 --- a/openspec/specs/p5-e11-rego-backend/spec.md +++ b/openspec/specs/p5-e11-rego-backend/spec.md @@ -97,7 +97,9 @@ the decision path itself. **Dependency order**: S01 → S02 → S03 → S04 → S05 → S06 → S07 → S08 → {S09, S10} → S11 → S12 → S13. **Do first: S01** — the ceiling document determines whether the backend's shape is right; building it without one reproduces the speculative-generality risk D-012 existed to -prevent. +prevent. **S04 is blocked on an operator answer** (judgment call (d)): the rule-7 boundary +question decides which package the evaluator lives in and which gate enforces it. S01–S03 are +unblocked and can run while that answer is pending. ## Judgment calls (decide-and-log / operator) @@ -108,6 +110,17 @@ silently ignore a rule it cannot evaluate). This is backward-compatible and deli forward-**in**compatible, and S02 records it in `API_STABILITY.md` as an announced additive change — no `apiVersion` bump. +(b1) **⚠️ SPIKE S06 BEFORE S02 — it is a serial chokepoint on an API that may not exist.** +Adversarial review, marked verify-not-verified: OPA's public `rego` package may bound +evaluation only via `context.Context` cancellation, with **no supported deterministic +instruction/step budget**. If so, judgment call (b) below cannot be satisfied as written — and +the current order (S05→S06→…) would stall the epic at story 6 of 13 with the **schema already +changed (S02)** and the **OPA dependency already added (S03)**. Mitigation, in order of +preference: (i) spike S06's feasibility before S02 lands anything irreversible; (ii) if no +deterministic budget exists, the fallback is that Rego-backed rules are restricted to +`phase: observe` — they report but never gate — so the epic still lands something honest; (iii) +what is **not** acceptable is relaxing (b). + (b) **DECIDED — evaluation is bounded by a deterministic budget, never a wall-clock timeout.** Per rule 7: the bound is an OPA evaluation-step/instruction budget that yields the identical outcome on any machine. A wall-clock deadline is permitted **only** as an outer backstop that @@ -123,15 +136,45 @@ required obligation. A Rego-backed obligation is proven only by an explicit, str value; absence of violations satisfies **non-obligation** rules only. S07 owns both polarities and must test the failing one. -(d) **🟡 OPERATOR — the OPA dependency is a supply-chain decision, not just an import.** -`github.com/open-policy-agent/opa` is a large dependency with a large transitive tree, on a -project whose release story includes SLSA-grade provenance, cosign signing, `govulncheck`, and -Scorecard. Adding it materially changes binary size, vulnerability surface, and the -`renovate`/`govulncheck` maintenance load. Recommended default: **accept**, since a -hand-rolled Rego evaluator would be far worse, and pin + vendor-audit it in S03. Flagged for -an explicit operator ack because it is the kind of change D-012's philosophy ("no speculative -frozen contracts for tiers without users") exists to make deliberate. **Recorded as D-141's -open sub-question.** +(d) **🔴 OPERATOR — BLOCKING. Adopting OPA narrows AGENTS.md rule 7's *mechanism*, and both +existing purity gates would miss it.** This is the sharpest finding of the design session and +it must be decided before S03, not discovered during it. + +*The gap, verified:* `internal/core/purity_test.go` parses each guarded file and flags +**that file's own** imports (`math/rand`, `crypto/rand`, `net`, `net/*`) and selectors +(`os.Getenv`, `time.Now`); `.golangci.yml`'s `pure-tree` depguard is `list-mode: lax`, +deny-only, over **direct** imports. **Neither is transitive.** So a file in +`internal/core/**` importing `github.com/open-policy-agent/opa/rego` passes both gates +green — while transitively linking `net/http` (OPA ships the `http.send` builtin), plus its +own clock and randomness use. The `net` deny exists precisely to encode "no network stack on +the decision path" (D-123), and OPA would defeat it invisibly. + +*Why S04's sandbox does not by itself resolve it:* the capability set makes `http.send` +uncallable **from policy**, which is the real threat. But the guarantee's *nature* changes +from "the network stack is not linked into the decision path" (structural, checkable by +grep) to "the network stack is linked but unreachable from policy" (behavioural, resting on +a capability file). That is a weaker guarantee, and it is a hard-rule change — it cannot be +made silently by a story. + +*Options:* +- **(d1) Accept the narrowing, explicitly [RECOMMENDED].** Rule 7's decision-path guarantee + becomes capability-enforced rather than link-enforced for the Rego tier. Requires an + **ADR-0011/rule-7 amendment** and a decision row — not just this spec. Pair it with + **extending the purity guard to a transitive check** (`go list -deps`) that asserts the + guarded tree's transitive closure contains no `net` **except** through the single, + explicitly allowlisted OPA path — so the exception is visible, pinned, and cannot widen to + a second dependency unnoticed. +- **(d2) Keep the guarded tree OPA-free** — evaluation behind an interface, implementation in + an unguarded package injected from `cmd/assent`. Honest about the boundary, but it moves + part of the decision path *outside* the tree rule 7 guards, which is arguably worse: the + guarantee is then neither link-enforced nor guard-covered. +- **(d3) Drop OPA.** A hand-rolled Rego evaluator would be far worse in every dimension. + Rejected unless the operator rejects (d1) and (d2). + +*Supply chain, separately:* OPA is a large dependency with a large transitive tree on a +project shipping cosign signing, SLSA-grade provenance, `govulncheck`, and Scorecard. It +materially changes binary size, vulnerability surface, and `renovate` load. S03 pins it and +records the size delta. **Both halves are recorded as D-141's open sub-question.** (e) **DECIDED — Rego modules are policy, and load from the target ref like all policy.** ADR-0010/ADR-0015's trust rules apply unchanged: a module is loaded from the target ref, @@ -189,9 +232,12 @@ human dependency. - Level: L0 - **REQ-E11-S02-02** — Given backward compatibility, when the full pre-E11 example and fixture corpus is validated against the new schema, then **every document still validates** - and no golden changes. - - Test: `examples/**`, `test/**` - - Verify: `task test && git diff --exit-code -- examples/ test/` + and no golden changes. **Both polarities are required**: a `oneOf` widening can also make a + previously-**rejected** document validate, so the `do-not-generalize` guards + (`schemas/testdata/compat/do-not-generalize/`, named as executable guards by + `API_STABILITY.md`) must still reject everything they rejected before. + - Test: `examples/**`, `test/**`, `schemas/testdata/compat/do-not-generalize/` + - Verify: `task test && go test ./schemas/... -run TestDoNotGeneralize && git diff --exit-code -- examples/ test/` - Level: L1 - **REQ-E11-S02-03** — Given `API_STABILITY.md:19`'s "announced-only for authored policy", when the schema changes, then `API_STABILITY.md` and the changelog record it as an announced @@ -237,11 +283,14 @@ human dependency. ### E11-S04 — OPA capability sandbox `[autonomous · engine-grade · maintainer LGTM]` -- **Dependencies**: S03. +- **Dependencies**: S03 **and the operator's answer to judgment call (d)** — S04 cannot close + without it, because (d1) and (d2) place the evaluator in different packages and gate it with + different mechanisms. Everything upstream of S04 (S01–S03) is unaffected and may proceed. - **Definition of done**: D-013's sandbox is real — a capability set that **denies by default** and allows an explicit, enumerated builtin list; `http.send`, `net.*`, `opa.runtime`, `time.*`, `rand.*`, and any I/O builtin are unavailable; a module using one - fails to **compile**, not at runtime. + fails to **compile**, not at runtime; and the rule-7 boundary question is closed by an ADR + amendment + decision row rather than by a green-but-non-transitive purity walk. - **REQ-E11-S04-01** — Given rule 7 and D-013, when a module calls `http.send`, `net.lookup_ip_addr`, `time.now_ns`, `rand.intn`, or `opa.runtime`, then compilation **fails** with a message @@ -256,12 +305,25 @@ human dependency. - Test: `internal/core/policy/testdata/allowed-builtins.golden` - Verify: `go test ./internal/core/... -run TestAllowedBuiltinsGolden` - Level: L1 -- **REQ-E11-S04-03** — Given `TestCorePurity`, when Rego evaluation lives in `internal/core`, - then the purity test still passes and the sandbox is what makes that true by construction — - if evaluation cannot be made pure, it moves out of core rather than weakening the test. - - Test: `internal/core/` purity test - - Verify: `go test ./internal/core/... -run TestCorePurity` +- **REQ-E11-S04-03** — Given judgment call (d) and the verified gap that **both purity gates + are direct-import/direct-call only**, when Rego evaluation is placed, then the placement + follows the operator's answer (d1/d2/d3) and the story **fails** if it lands a green + `TestCorePurity` that is green only because the walk is non-transitive. Under **(d1)**: the + purity guard is extended to a transitive `go list -deps` assertion over the guarded tree, + with the OPA path as the single named, pinned exception, and a **mutation control** proving + the new check goes red when a second `net`-reaching dependency is added. Under **(d2)**: a + depguard rule denies the OPA package from every guarded directory, with a mutation control + proving it fires. + - Test: `internal/core/purity_test.go`, `.golangci.yml`, `hack/lint/depguard_test.sh` + - Verify: `go test ./internal/core/... -run TestCorePurity && task lint` - Level: L1 +- **REQ-E11-S04-04** — Given (d1) changes a hard rule, when that option is chosen, then an + **ADR amendment** (ADR-0011 / AGENTS.md rule 7) and a `D-nnn` row land **before** S05, both + stating plainly that the decision path's network guarantee became capability-enforced rather + than link-enforced for the Rego tier. The story cannot close on spec text alone. + - Test: `docs/adr/`, `docs/decisions/decisions.md` + - Verify: manual — S05 is blocked until present + - Level: L0 ### E11-S05 — Input binding: the identical `EvaluationInput` `[autonomous · engine-grade]` From 94907f79ff69e52f3a5c9ab9d5543958f1d04a74 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 15:34:17 +0200 Subject: [PATCH 4/8] :wrench: chore(release): regenerate CHANGELOG.md for the E10/E11 design session --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fcd08fa..1743499c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,9 @@ repository still gets a decision, never by following the link; no release carrie ### Documentation - :memo: docs(decisions): record D-126 (D-031 domain deferred) and D-127 (SEC-05 PAT rotation scheduled) +- :memo: docs(decisions): unlock E10 + E11 (D-140/D-141) and record ADR-0021 +- :memo: docs(openspec): decompose E10 and E11 into INVEST stories +- :memo: docs(openspec): fix two P0 design defects found by adversarial review ### Testing - :white_check_mark: test(release): anchor the D-120 note check on its header sentence, not the bare token From d6ab49224c298d89e324567327a4ef7f04d57204 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 15:36:54 +0200 Subject: [PATCH 5/8] :memo: docs(openspec): correct the E10 story count to 19 after adding S00 --- docs/decisions/decisions.md | 2 +- openspec/specs/backlog.md | 2 +- openspec/specs/later-phases.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index c5635b61..1a82ef84 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -144,5 +144,5 @@ project/process decisions. | D-137 | 2026-08-09 | **REL-14 — `cliff.toml` groups by the CONVENTIONAL TYPE after the gitmoji shortcode, not by the emoji; a real hotfix was published under "Other".** The parser list matched eight shortcodes (`:sparkles: :bug: :memo: :recycle: :white_check_mark: :lock: :wrench: :tada:`) and, as alternatives inside the same rules, conventional types anchored at the START of the subject (`^fix`, `^ci`, `^docs`, …). This project always writes the shortcode FIRST, so **those `^type` alternatives could never fire** — they were dead regex from the day the file was written — and every subject whose shortcode was outside the eight fell through the `.*` catch-all into **Other**. Measured on the rendered v0.2.0 Release body: `:ambulance: fix(forge): skip malformed bot markers with a warning instead of bricking reconcile (AUD-S12, REL-06)` — a fix an adopter would go looking for under **Fixes** — sat in Other, next to 18 `ci(...)` commits, 26 `test(...)`, 6 `feat(...)`, 5 `style(...)`, 4 `specs(...)`, 3 `docs(...)`, 2 `refactor(...)` and 2 `chore(...)`. **Fix:** eight new parser entries, placed AFTER the eight shortcode rules and BEFORE the `.*` catch-all, keyed on `^:[a-z0-9_]+: [(:]` — the type the author declared — plus one shortcode alias, `:test:` → Testing (a typo'd shortcode; no such gitmoji exists, and all 19 uses are tests, including one whose type field is the equally typo'd `render(...)`). Placement is deliberate and narrow: putting the type tier FIRST would re-file large parts of the eight mapped shortcodes too; placing it after means it only sorts what the catch-all was already catching. **Keyed on type, never on the emoji, because the emoji is the unreliable half** — `:lipstick: fix(provider): …` is a fix, not a UI change, and `:art:` is used for both `style(…)` and `refactor(…)`; a mapping by emoji dictionary would have mis-filed both. The inventory was derived from `git log --format=%s` over the FULL history (30 distinct shortcodes, 22 of them unmapped), not from a sample, and **every** unmapped-shortcode subject in this repo turned out to declare a conventional type, which is what makes type-keying total rather than lucky. **Judgment calls, stated because a wrong group is worse than Other:** (a) `specs(…)` → Documentation — spec authoring under `openspec/` is a written artifact, the same class the existing `:memo:|^docs` rule files; (b) `style(…)` → Refactoring — internal code hygiene with no behaviour change, which is what that group already means, and closer than Other; (c) `revert(…)` → **left in Other on purpose**: no existing group fits it and adding a Reverts group is a changelog-structure change, not a categorisation fix; (d) one malformed subject, `:test(release): add CI audit gate for single CodeQL workflow`, declares no parseable type and stays in Other — the commit is published history and cannot be reworded. Those two lines are the entire remaining Other. **Effect on already-published sections, stated as a MULTISET because the raw line diff is misleading:** the rendered bullet multiset is **identical** before and after — 509 unique bullets, 514 rendered lines, nothing added, nothing dropped — and 69 unique bullets move, all in one direction, all out of Other: Testing 28, Chores 20, Documentation 7, Refactoring 7, Features 5, Fixes 2. Zero bullets move between two non-Other groups. This re-files lines inside the already-published `[0.1.0]` and `[0.0.0]` sections; acceptable on the same ground D-136 established and the review accepted — `CHANGELOG.md` is a derived artifact regenerated in full from history, and the published v0.1.0 GitHub Release body is a separate immutable artifact that is unaffected. **Proof, both polarities, in the already-wired `release-changelog-gate-test`:** `hack/release/changelog_gate_test.sh` §8 asserts the `:ambulance:` hotfix renders under Fixes and — structurally, so it keeps holding as history grows — that NO line under Other declares a type this repo files; §8a strips the `# REL-14`-tagged entries (mutation proven to have landed by line count), shows the hotfix falls back to Other, and asserts the rendered subject multiset is unchanged by the re-grouping, so the parsers can only re-file and never add or drop a line. §7a's "changes nothing else" claim was restated over the same multiset in this lane, because removing a group's last member also removes its `###` heading and a raw line diff reads that as unexplained churn. Revert: drop the `# REL-14` entries and §8/§8a — the hotfix returns to Other. | | D-138 | 2026-08-09 | **The three reliability P1s of the 2026-08-09 audit (RELI-01/02/03) are DEFERRED past v0.2.0, deliberately and with the deferral recorded (AGENTS.md rule 6).** All three are **pre-existing at v0.1.0**; nothing in v0.2.0 introduced or worsened any of them, verified against `1d8aa60` (`origin/main` at audit time). Holding the tag would delay two fail-open fixes and a P0 in order to fix defects that are already live. **The machine direction holds in all three:** no probed path merges anything unevaluated — `MergeCAS` re-checks all three pins atomically, arming stays default-deny (`internal/forge/precondition.go`), and the `.assent/**` self-edit guard still yields BLOCK with zero forge writes. What fails is the **human signal channel**, which is why they are P1 and not P0. **RELI-01 — clean exit-0 runs leave a stale APPROVE banner, and D-130's compensating control does NOT apply here.** `cmd/assent/run.go` emits the DecisionRecord at step 8 and only then enters the step-9 switch, whose two GUARD branches (`reservedSelfEditBlock`, `untrustedExecutionContext`) skip `forge.Reconcile` **entirely** — including the summary upsert. So run 1 APPROVEs and posts `✅ Decision: APPROVE`; run 2, after a `.assent/**` edit is added, decides BLOCK, exits **0**, and leaves that note byte-identical. `openspec/specs/p5-e5-provider-host/spec.md` REQ-E5-S08-03 accepts a stale banner on the abort path precisely because "a REVIEW rerun upserts that same summary to REVIEW and adds an unresolved discussion" — the discussion being what holds the merge under `only_allow_merge_if_all_discussions_are_resolved`. **On the GUARD-1 self-edit path no thread is posted at all**, so nothing holds the merge and no later run repairs the banner: the compensating control the spec relies on is absent exactly where it is needed. Reachability is ordinary CI cadence, and it is deliberately triggerable at the cost of two pushes — D-042's own threat model rendering as a green tick. **Its fix is out of release scope BY CONSTRUCTION**, not by preference: `openspec/specs/p5-aud-audit-remediation/spec.md` pins "the decision is BLOCK with zero forge writes (GUARD-1 dominance over the gap-degrade)", so upserting a summary on that path REOPENS a frozen acceptance criterion and needs an openspec change proposal first (see OQ-31). **RELI-02 — a duplicated summary comment is UNRECOVERABLE WITHOUT A HUMAN.** `reconcileThread` has both `repairDuplicates` and a step-9 rescan; `reconcileSummary` (`internal/forge/forge.go`) has neither — it is a bare `UpsertComment`. No branch of `Reconcile` can remove a second summary note, so once two exist the wrong one can sit at `decision=APPROVE` forever. Not a corner case: `PreconditionFromCapabilities` seeds `DuplicatePrevention` to `unserialized-best-effort` because per-MR `resource_group` serialization is never probed, i.e. the race is the project's **declared default posture**. First item of v0.2.1. **RELI-03 — the standing bot approval has no retraction and the cited mitigation is never probed.** `reconcileApproveMerge` records `Approve` before `MergeCAS`; on CAS failure in the TOCTOU window the approval is already written and `forge.Forge` has no `Unapprove` verb. The code's own safety argument named the forge's remove-approvals-on-push setting — and **`reset_approvals_on_push` appears in ZERO Go files**: nothing reads it, `probeCapabilities` does not fetch it, `assent doctor` cannot report it. `spike-secure-setup.md` D11 specified refuse-to-arm when it is off and C19 specified doctor verification; **neither was built**, and the comment's deferral pointer named P4-E1-S10 — a slice that SHIPPED (D-041), so the concern was never picked up. **The comment is corrected in this lane** (`internal/forge/forge.go`, text only, no behaviour change) because an asserted-and-unbuilt mitigation TERMINATES THE SEARCH — it is how this survived two prior audits. RELI-03 needs no open question: D11 already decided it; it is unbuilt, not undecided. **Also recorded here, same shape — the ADR-0019 `duplicate_prevention:` MUST is unmet.** The value is computed and typed through to `PreconditionReport` and then never printed: `emitDoctorReport` emits only the arm verdict and refusal reasons, so no `assent doctor` output contains the string. The safe-default half DOES hold (nothing can claim `single-writer-serialized` without the mechanism). **Judgment call: annotate, do not emit.** Emitting is ~3 lines, but it is one instance of audit **ARCH-11** — doctor computes a typed capability report and prints essentially none of it — so emitting this one field would half-close ARCH-11 and leave the report inconsistent with itself, and it is a user-visible CLI output change, which does not belong in a docs-truth lane before a tag. ADR-0019 now carries the unmet-MUST annotation and points here; the emission belongs to the v0.2.1 ARCH-11 slice with its own tests. **Not deferred silently:** all three plus ARCH-11 are named in the v0.2.1 wave. Revert: `git revert` this lane — restores the false RELI-03 comment and changes no behaviour, since the lane changes none. | | D-139 | 2026-08-09 | **The security lens's SEC-01/SEC-04/SEC-05 are KNOWN and DEFERRED to v0.2.1; the tag proceeds, but the release must not claim `--checkout` is now safe.** All three are pre-existing at v0.1.0 and none was introduced by v0.2.0 (verified at `1d8aa60`). Reach on SEC-01 is bounded and that is why it does not block: **no Go non-test code constructs `--checkout`**, no CI template, example, hack script or e2e passes it, and neither `README.md` nor `docs/usage/walkthrough.md` mentions it at all — the adoption path this project actually teaches is checkout-less. The fix is also monotone-safer to ship after the tag, since the P0 already merged in this release was likewise `--checkout`-only. **SEC-01 — the `--checkout` tree is never bound to the evaluated SHA.** With `--checkout` the local tree is the sole authority for the judged bytes and the changed-file set (D-077), while `pins.sourceSha`/`targetSha` and the compare-and-swap come from the forge; **nothing compares the two**. Measured: `cmd/assent/checkout.go` contains zero occurrences of `SHA`/`Sha`, and `run.go` has no step relating the tree to a commit. Reproduced by the lens: forge head a destructive shrink, local checkout a benign grow → `APPROVE`, `approvals=1 merges=1`. ADR-0015 §2 promises every write re-verifies what it acts on; the guard re-verifies that the *metadata* has not moved, never that the judged *bytes* are the bytes at that SHA. **Condition attached and DISCHARGED IN THIS LANE:** this release hardens `--checkout` extensively (D-133 refuses any symlink anywhere) and `docs/usage/cli.md` already named "run without `--checkout`" as remedy #1, so the page read as though the flag had been made sound. A *Known limitation: the checkout is not bound to the evaluated commit* section now says otherwise, framed as a property of how the checkout is CONSTRUCTED (build `head/` from the MR head SHA; cancel superseded pipelines) and **claiming no mitigation on assent's part** — `auto_cancel_redundant_pipelines` is a project setting the tool never probes, and citing an unread setting as a control is the exact pattern this audit found three times. **Named fix for v0.2.1:** bind the checkout to the evaluated SHA, or cross-check the local changed-file set against the already-fetched `snapshot.ChangedFiles` and fold divergence opaque. **SEC-04 — protected-config verification is a substring test.** `internal/forge/gitlab/snapshot.go` sets `caps.ProtectedPipelineExternal = strings.Contains(proj.CIConfigPath, "@")`, while ADR-0015 §4 calls protected config THE load-bearing adoption prerequisite and says doctor refuses to arm when it cannot verify it. Reproduced: an in-repo, author-editable `.ci/pipeline@v2.yml` arms and merges. **Named fix:** replace the substring test with a structural parse — require a non-empty suffix after `@` that contains `/`. **SEC-05 — absent provenance is read as trusted.** `snapshot.go` computes `ForkMR: mrResp.SourceProjectID != 0 && mrResp.SourceProjectID != mrResp.ProjectID`, so an absent or zero `source_project_id` yields `ForkMR=false` and the fork guard never engages; reproduced to `approvals=1 merges=1`. **Named fix:** treat absent or zero `source_project_id` as **fork** (untrusted) — unknown provenance must fail toward advisory-only. **The composition worth not losing, recorded AS A HYPOTHESIS, not as measured:** SEC-05 × SEC-03 would let a fork MR reach the write path and then leave a **standing bot approval on a fork MR**, converting ADR-0015 §8's "CI on fork / untrusted-contributor MR → advisory-only, no writes" into "we can leave an approval on a fork". There is **no evidence real GitLab ever omits `source_project_id`**; the composition is untested and is written down so the v0.2.1 SEC-05 fix is understood as closing more than a provenance nicety. **Correction carried from the lens:** its SEC-08 was **WITHDRAWN as non-novel** — D-130 already documents the host-declaration `continue`-on-any-error as a knowingly-deferred sibling. It is not a new finding and must not be written up as one. | -| D-140 | 2026-08-10 | **E10 (GitHub adapter + Actions entrypoint) is UNLOCKED by direct operator instruction; ADR-0021 governs the seam and `openspec/specs/p5-e10-github-forge/spec.md` decomposes it into 18 stories.** E10 was **Locked** under D-012 ("unlocks with a named consumer"), and that lock was reaffirmed twice — D-017 named the consumer and still said "E10/E13 stay locked", D-019 restated "GitHub + remote packs locked". No prior decision authorized it, so the operator's instruction this session is a NEW unlock event, recorded here BEFORE any spec text or code exists (AGENTS.md rule 6). **What this authorizes**: the GitHub forge adapter, the multi-adapter seam work it depends on, and forge selection in `run`/`doctor`. **What it does NOT authorize**: E13 remote packs (still Locked under D-012 — the "named consumer" reasoning is untouched for that tier), E12 `serve`, E14 CRD, or any third forge / plugin-forge protocol (ADR-0021 Option D, rejected — no named consumer). **Scope of v1 GitHub**: behavioural parity for the GATE, not device-for-device parity (dossier §3, OQ-7/OQ-18 — required-conversation-resolution carries acknowledgement, `REQUEST_CHANGES` reserved for block); the three known deltas (review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, and where GitHub cannot prove what the gate needs the adapter reports the gap and NEVER arms — the same shape as GitLab free tier. **Why a seam epic and not just an adapter**: the 2026-08-09 audit's ARCH-18/ARCH-19 found `docs/planning/design-notes/e10-forge-port-lift.md` under-scopes the epic, and that the conformance suite cannot be run by a second adapter because all ~1,166 lines live in `_test.go` files Go cannot import — so a GitHub adapter written today would be developed against no executable contract and D-084's `github-deferred` catalog rows would be unflippable by construction. The original ARCH-18/ARCH-19 finding text is NOT in the repo (only the one-line summary at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives); the two design buckets were therefore RE-DERIVED as (A) no neutral capability model — the GitHub dossier §4 names eleven flags the port needs while `probeCapabilities` reads three project fields and `capabilityGap` is computed in GitLab terms — and (B) no port-level transport/auth policy — GitHub thread resolution is GraphQL-only and needs PAT-vs-App auth, while bounded reads/pagination caps (AUD-S10) and idempotent-GET retry/deadlines (AUD-S11) live inside the GitLab client. Both are recorded as a re-derivation, not as a citation. **OPEN SUB-QUESTION (operator)**: whether the Actions entrypoint (E10-S16) stays in scope — `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", but the action is packaging on top of an adapter and is the one story whose absence leaves everything else useful; default is to keep it last and independently droppable. **Consequence to watch (E10-S04)**: forcing every capability to be stated explicitly may turn a GitLab arming path that passes today into an honest capability gap — a user-visible behaviour change that gets its OWN decision row and changelog entry, never absorbed silently into "E10 refactor". Revert: re-lock E10 in `later-phases.md`, delete the spec directory; nothing here is published API (`forge.RunPort` is `internal/`), so no compatibility window applies. | +| D-140 | 2026-08-10 | **E10 (GitHub adapter + Actions entrypoint) is UNLOCKED by direct operator instruction; ADR-0021 governs the seam and `openspec/specs/p5-e10-github-forge/spec.md` decomposes it into 19 stories (S00, the addressing & representation model, was added after an adversarial review found two P0 representation defects).** E10 was **Locked** under D-012 ("unlocks with a named consumer"), and that lock was reaffirmed twice — D-017 named the consumer and still said "E10/E13 stay locked", D-019 restated "GitHub + remote packs locked". No prior decision authorized it, so the operator's instruction this session is a NEW unlock event, recorded here BEFORE any spec text or code exists (AGENTS.md rule 6). **What this authorizes**: the GitHub forge adapter, the multi-adapter seam work it depends on, and forge selection in `run`/`doctor`. **What it does NOT authorize**: E13 remote packs (still Locked under D-012 — the "named consumer" reasoning is untouched for that tier), E12 `serve`, E14 CRD, or any third forge / plugin-forge protocol (ADR-0021 Option D, rejected — no named consumer). **Scope of v1 GitHub**: behavioural parity for the GATE, not device-for-device parity (dossier §3, OQ-7/OQ-18 — required-conversation-resolution carries acknowledgement, `REQUEST_CHANGES` reserved for block); the three known deltas (review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, and where GitHub cannot prove what the gate needs the adapter reports the gap and NEVER arms — the same shape as GitLab free tier. **Why a seam epic and not just an adapter**: the 2026-08-09 audit's ARCH-18/ARCH-19 found `docs/planning/design-notes/e10-forge-port-lift.md` under-scopes the epic, and that the conformance suite cannot be run by a second adapter because all ~1,166 lines live in `_test.go` files Go cannot import — so a GitHub adapter written today would be developed against no executable contract and D-084's `github-deferred` catalog rows would be unflippable by construction. The original ARCH-18/ARCH-19 finding text is NOT in the repo (only the one-line summary at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives); the two design buckets were therefore RE-DERIVED as (A) no neutral capability model — the GitHub dossier §4 names eleven flags the port needs while `probeCapabilities` reads three project fields and `capabilityGap` is computed in GitLab terms — and (B) no port-level transport/auth policy — GitHub thread resolution is GraphQL-only and needs PAT-vs-App auth, while bounded reads/pagination caps (AUD-S10) and idempotent-GET retry/deadlines (AUD-S11) live inside the GitLab client. Both are recorded as a re-derivation, not as a citation. **OPEN SUB-QUESTION (operator)**: whether the Actions entrypoint (E10-S16) stays in scope — `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", but the action is packaging on top of an adapter and is the one story whose absence leaves everything else useful; default is to keep it last and independently droppable. **Consequence to watch (E10-S04)**: forcing every capability to be stated explicitly may turn a GitLab arming path that passes today into an honest capability gap — a user-visible behaviour change that gets its OWN decision row and changelog entry, never absorbed silently into "E10 refactor". Revert: re-lock E10 in `later-phases.md`, delete the spec directory; nothing here is published API (`forge.RunPort` is `internal/`), so no compatibility window applies. | | D-141 | 2026-08-10 | **E11 (Rego complex-rule backend) implementation is UNLOCKED by direct operator instruction; `openspec/specs/p5-e11-rego-backend/spec.md` decomposes it into 13 stories under ADR-0002 v2.** E11's CONTRACT was already unlocked by D-017; what was gated was IMPLEMENTATION, twice: "after Phase 4" (satisfied — the Phase-4 adoption gate closed with D-042) and, per D-017, **evidence-based per rule** ("each ported rule tries CEL first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling"). This row records the operator lifting that per-rule evidence gate. **What it does NOT waive**: the DESIGN need the gate was protecting — E11-S01 still requires a written tier-1 ceiling document with concrete rules per shape (multi-pass, cross-manifest, set-difference, graph-relationship), and any shape found CEL-expressible is struck from scope. **What it does NOT authorize**: WASM or gRPC predicate backends (still Locked under D-012 — this unlocks Rego only); domain-aware joins and in-process Go rule plugins (D-017 DECLINED both permanently — not deferred); giving Rego any control over aggregation, effects, or points (ADR-0002 v2 boundary); any `EvaluationInput` change. **Two constraints found during design that shape the epic**: (1) **E11 is the first epic whose DoD is `git diff schemas/` != 0.** P3-E1-S02's backend-neutrality guarantee ("no field naming a predicate backend anywhere in the schema") applies to `EvaluationInput` and HOLDS — no decision contract changes — but `schemas/policy/v1alpha1/merge-policy.schema.json` defines the predicate leaf as `additionalProperties:false, required:["cel"]`, so a `rego:` leaf IS a policy-schema change. `API_STABILITY.md:19` permits exactly this within `v1alpha1` as an announced additive change with an openspec change and no `apiVersion` bump; the change is backward-compatible and deliberately forward-INcompatible (an older binary rejects a `rego:` leaf by strict-decode, which is the correct direction — it must not silently ignore a rule it cannot evaluate). A reviewer applying the previous epics' `git diff schemas/ == 0` habit will flag the correct change as a violation; E11-S02-04 scopes the drift guard rather than deleting it. (2) **The reflexive safety measure violates AGENTS.md rule 7.** Rego ships `time.now_ns()`, `rand.intn()`, and `http.send()`, all of which are denied structurally by the D-013 capability sandbox (E11-S04, compile-time failure, golden allowlist so an OPA upgrade cannot widen it unnoticed) — but bounding evaluation with a WALL-CLOCK TIMEOUT would itself be a rule-7 violation, making the same policy over the same ChangeSet decide differently on a slow runner. E11-S06 therefore requires a machine-independent evaluation budget, and exceeding it must be a PROCESS ERROR that can never be a policy outcome; "timeout → BLOCK" is explicitly rejected as machine-dependent while merely LOOKING fail-closed. **Also fixed by the spec**: zero violations NEVER proves a required obligation (`later-phases.md`'s explicit polarity rule; E11-S07-02 tests the failing polarity). **🔴 BLOCKING OPEN SUB-QUESTION (operator) — adopting OPA narrows rule 7's MECHANISM, and both existing purity gates would miss it.** Verified during the design session: `internal/core/purity_test.go` flags only each guarded file's OWN imports (`math/rand`, `crypto/rand`, `net`, `net/*`) and selectors (`os.Getenv`, `time.Now`), and `.golangci.yml`'s `pure-tree` depguard is `list-mode: lax`, deny-only, over DIRECT imports — **neither is transitive**. A file in `internal/core/**` importing `github.com/open-policy-agent/opa/rego` therefore passes both gates GREEN while transitively linking `net/http` (OPA ships the `http.send` builtin), defeating the `net` deny that encodes D-123 / rule 7 invisibly. S04's capability sandbox makes `http.send` uncallable FROM POLICY — the real threat — but the guarantee's nature changes from "the network stack is not linked into the decision path" (structural, greppable) to "linked but unreachable from policy" (behavioural, resting on a capability file). That is a hard-rule change and cannot be made by a story. Options: **(d1)** accept the narrowing with an ADR-0011/rule-7 amendment plus a transitive `go list -deps` purity check allowlisting exactly the OPA path (RECOMMENDED); **(d2)** keep the guarded tree OPA-free by injecting an evaluator from `cmd/assent` — honest, but it moves part of the decision path outside the tree rule 7 guards; **(d3)** drop OPA (a hand-rolled evaluator would be far worse; rejected unless d1 and d2 are). **E11-S04 is blocked on this answer** (it decides the evaluator's package and its gate); S01–S03 are unblocked. Separately on supply chain: OPA is a large dependency with a large transitive tree on a project shipping cosign/SLSA provenance, `govulncheck`, and Scorecard — recommended default is accept-and-pin, with S03 recording the binary-size delta. Revert: re-assert the D-017 per-rule evidence gate, delete the spec directory, leave the `# locked: D-012` quarantine marker in place. | diff --git a/openspec/specs/backlog.md b/openspec/specs/backlog.md index 3236de3e..f85b9a20 100644 --- a/openspec/specs/backlog.md +++ b/openspec/specs/backlog.md @@ -614,7 +614,7 @@ Epic paragraphs (goal, ADR constraints, exit gate, story seeds) in | --- | --- | --- | | 3 — Contracts first | P3-E1 schemas + contract fixture (incl. ApprovalEvidence + named-consumer fixture) · P3-E2 versioning/compat spec · P3-E3 example migration · P3-E4 lifecycle: phase/profiles/comparison (ADR-0018) · P3-E5 publication reconciliation protocol (ADR-0019) | strict end-to-end contract fixture validates (ADR-0017 §8, D-016); new ADRs 0018/0019 accepted at the freeze review | | 4 — Walking skeleton | P4-E1 (+ rerun-idempotence gate, D-017) · **P2-E4-NS (OQ-24 timed run)** · holdout adjudication (OQ-25) | L3 skeleton green + **one real repo on live MRs** (D-012); north-star wording only after timed run | -| 5 — Implementation | E1–E7 **DONE**; **E7 AUTONOMOUS COMPLETE** (S01–S05+S08, D-087); **E8 AUTONOMOUS COMPLETE** ([p5-e8-renderer/spec.md](p5-e8-renderer/spec.md), S01–S14, D-098); **E9 AUTONOMOUS COMPLETE** ([p5-e9-distribution/spec.md](p5-e9-distribution/spec.md), S01–S13, D-099–D-111 CLOSED; Homebrew Formula live; PAT rotate optional); **PCS AUTONOMOUS COMPLETE** ([p5-pcs-policy-comparison/spec.md](p5-pcs-policy-comparison/spec.md), S01–S09, **D-057 closed**, D-118); **E10 UNLOCKED + DECOMPOSED** (D-140, [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md), 18 stories, ADR-0021); **E11 IMPLEMENTATION UNLOCKED + DECOMPOSED** (D-141, [p5-e11-rego-backend/spec.md](p5-e11-rego-backend/spec.md), 13 stories); E12 **contract-unlocked** (D-017), not decomposed; E14 gated on Spike D; **E13 still locked** (D-012) | per-epic; E9 exit = tagged signed release + docs live + brew Formula (D-111); PAT rotate optional | +| 5 — Implementation | E1–E7 **DONE**; **E7 AUTONOMOUS COMPLETE** (S01–S05+S08, D-087); **E8 AUTONOMOUS COMPLETE** ([p5-e8-renderer/spec.md](p5-e8-renderer/spec.md), S01–S14, D-098); **E9 AUTONOMOUS COMPLETE** ([p5-e9-distribution/spec.md](p5-e9-distribution/spec.md), S01–S13, D-099–D-111 CLOSED; Homebrew Formula live; PAT rotate optional); **PCS AUTONOMOUS COMPLETE** ([p5-pcs-policy-comparison/spec.md](p5-pcs-policy-comparison/spec.md), S01–S09, **D-057 closed**, D-118); **E10 UNLOCKED + DECOMPOSED** (D-140, [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md), 19 stories, ADR-0021); **E11 IMPLEMENTATION UNLOCKED + DECOMPOSED** (D-141, [p5-e11-rego-backend/spec.md](p5-e11-rego-backend/spec.md), 13 stories); E12 **contract-unlocked** (D-017), not decomposed; E14 gated on Spike D; **E13 still locked** (D-012) | per-epic; E9 exit = tagged signed release + docs live + brew Formula (D-111); PAT rotate optional | Named-consumer disposition (what unlocked, what stayed locked, and why): [docs/planning/named-consumer-compat.md](../../docs/planning/named-consumer-compat.md). diff --git a/openspec/specs/later-phases.md b/openspec/specs/later-phases.md index 76d2b4df..d4c70da3 100644 --- a/openspec/specs/later-phases.md +++ b/openspec/specs/later-phases.md @@ -262,7 +262,7 @@ Was Locked (D-012), reaffirmed locked by D-017 and D-019; unlocked by direct ope instruction (D-140), not by the named-consumer trigger. Seam kept honest by the P1-E3-S03 dossier (REQUEST_CHANGES + conversation-resolution parity, merge queue as merge-result pin, base-ref workflow trust) and by the conformance suite being forge-neutral (E7). -**18 stories (S01–S17 autonomous, S18 infra-gated)** governed by **ADR-0021**, which decides +**19 stories (S00–S17 autonomous, S18 infra-gated)** governed by **ADR-0021**, which decides the three things the audit found under-scoped (ARCH-18/ARCH-19): the named `forge.RunPort` composite port, an **importable** conformance suite (today all ~1,166 lines are in `_test.go` files a second adapter cannot import), a neutral capability model where `unknown` never arms, From 4cb593e1dfcbdc57870e89c47e8aa196cf7a7e4d Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 15:36:55 +0200 Subject: [PATCH 6/8] :wrench: chore(release): regenerate CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1743499c..fe43aa1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,7 @@ repository still gets a decision, never by following the link; no release carrie - :memo: docs(decisions): unlock E10 + E11 (D-140/D-141) and record ADR-0021 - :memo: docs(openspec): decompose E10 and E11 into INVEST stories - :memo: docs(openspec): fix two P0 design defects found by adversarial review +- :memo: docs(openspec): correct the E10 story count to 19 after adding S00 ### Testing - :white_check_mark: test(release): anchor the D-120 note check on its header sentence, not the bare token From f9d5ccabd78dd3695ce4d40383a088f2785a9594 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 16:33:09 +0200 Subject: [PATCH 7/8] :memo: docs(openspec): fix 3 P1 design defects found by independent review of PR #45 F1 -- the fork-PR P0 fix was half-landed. ADR-0021 item 1 still froze FileAtRef(project, path, ref) as the port's content accessor while item 5 said the port stops addressing by (project, branch-name); REQ-E10-S02-01 and -05 inherited the contradiction, so an implementer could satisfy -01 literally and preserve the fabricated-DELETE defect with a green build. Reconciled by stating what the narrowing actually binds: the GOVERNED SUBJECT (run.go:270,274) moves to FileAtBase/FileAtHead; the POLICY loads (run.go:203,211,253) deliberately keep FileAtRef because ADR-0015 s1 requires them to read the protected target ref by name -- migrating those would be a trust-boundary regression, not a cleanup. Both directions are now asserted. F2 -- E11's ordering was stated three incompatible ways and the backlog carried an E11-S06' row with no story section, no DoD and no REQ IDs (14 rows against a stated 13 stories). Promoted to a real story E11-S00, the deterministic-budget feasibility spike, built in a nested throwaway module so it answers the question without adding OPA to the root go.mod (no go.work exists, so Go excludes it from ./...). E11 is now 14 stories, S00-S13, and S02 is blocked on S00 because an observe-only rego: leaf is a different published contract from a gating one. F3 -- the spec said E11-S01..S03 were unblocked while judgment call (d) was pending. False, and the most consequential of the three: S03 is the story that EFFECTS the narrowing (d) governs -- it adds OPA to go.mod and its test paths sit in the D-123 guarded tree. Because neither purity gate is transitive, S03 would have landed green while converting rule 7's guarantee from link-enforced to capability-enforced, i.e. narrowing a hard rule by merging a story. S03 is now blocked on (d) alongside S04, and gains REQ-E11-S03-04 requiring the transitive purity guard under (d1). Also: recorded the operator's two 2026-08-10 answers in D-140/D-141 rather than leaving them only in the gitignored INBOX (rule 6), and made explicit that 'accept and pin OPA' settles (d)'s supply-chain half only -- (d2) also accepts and pins OPA, so the mechanism half is still open and is what blocks S03. Corrected Verify commands naming gates that do not exist (task scrub), packages that do not exist (internal/core/{lint, catalogue,testharness}), a shell drift guard that is actually Go (internal/schemadrift), a guard task check never runs (check-migration-invariants.sh), and two false counts (1,166 -> 1,155 conformance lines; 15 -> 14 non-deferred catalog rows). --- docs/adr/0021-multi-adapter-forge-seam.md | 35 ++-- docs/decisions/decisions.md | 4 +- openspec/specs/backlog.md | 14 +- openspec/specs/later-phases.md | 6 +- openspec/specs/p5-e10-github-forge/spec.md | 64 ++++--- openspec/specs/p5-e11-rego-backend/spec.md | 195 +++++++++++++++++---- 6 files changed, 237 insertions(+), 81 deletions(-) diff --git a/docs/adr/0021-multi-adapter-forge-seam.md b/docs/adr/0021-multi-adapter-forge-seam.md index 1c50feec..1b12ea49 100644 --- a/docs/adr/0021-multi-adapter-forge-seam.md +++ b/docs/adr/0021-multi-adapter-forge-seam.md @@ -16,7 +16,7 @@ seam a second adapter must plug into is only half-built: `internal/forge` (`port.go`), but `cmd/assent`'s `forgePort` is still an anonymous interface literal declared at the call site, and `run.go` still calls `gitlab.SyntheticDigest` directly. `port.go`'s own scope note records both as E10 work. -2. **The conformance suite cannot be reused.** All 1,166 lines of +2. **The conformance suite cannot be reused.** All 1,155 lines of `internal/forge/conformance` live in `_test.go` files, 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. @@ -104,7 +104,15 @@ consisting of four committed pieces: `cmd/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)`. + `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. `FileAtRef` survives because *policy* is ref-addressed by contract: + ADR-0015 §1 requires the MergePolicy, RulesetBinding and pack to load from the **target + ref by name**, which `cmd/assent/run.go:203,211,253` does today and must keep doing. + `FileAtBase`/`FileAtHead` are the **governed subject's** only legal accessors. An adapter + that implements `FileAtBase` by delegating to `FileAtRef(project, path, sourceBranch)` + reintroduces the defect item 5 exists to kill. `cmd/assent` depends on `forge.RunPort` **only** — a depguard rule denies `cmd/assent` importing `internal/forge/gitlab` **and** `internal/forge/github`, replacing the current three-symbol allowlist. The merge-digest *scheme* is adapter-owned: @@ -129,14 +137,21 @@ consisting of four committed pieces: installation token) and protocol (REST vs. GraphQL) stay **adapter-internal freedom** — the port never names a transport. -5. **An explicit addressing model, decided before `FileAtRef` is frozen.** The port stops - addressing content 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/head` on - 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. Smuggling - `refs/pull/N/head` into `MRInfo.SourceBranch` is explicitly rejected: it corrupts a - documented field and leaks into rendering. +5. **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/head` on 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. Smuggling `refs/pull/N/head` into `MRInfo.SourceBranch` is 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:270,274`, the reads whose 404-maps-to-`nil` feeds + `change.OneSidedLifecycle` and mints the fabricated whole-file DELETE. The **policy** loads + at `run.go:203,211,253` are *deliberately* still `FileAtRef(project, path, targetBranch)`: + 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. diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index 1a82ef84..951891e8 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -144,5 +144,5 @@ project/process decisions. | D-137 | 2026-08-09 | **REL-14 — `cliff.toml` groups by the CONVENTIONAL TYPE after the gitmoji shortcode, not by the emoji; a real hotfix was published under "Other".** The parser list matched eight shortcodes (`:sparkles: :bug: :memo: :recycle: :white_check_mark: :lock: :wrench: :tada:`) and, as alternatives inside the same rules, conventional types anchored at the START of the subject (`^fix`, `^ci`, `^docs`, …). This project always writes the shortcode FIRST, so **those `^type` alternatives could never fire** — they were dead regex from the day the file was written — and every subject whose shortcode was outside the eight fell through the `.*` catch-all into **Other**. Measured on the rendered v0.2.0 Release body: `:ambulance: fix(forge): skip malformed bot markers with a warning instead of bricking reconcile (AUD-S12, REL-06)` — a fix an adopter would go looking for under **Fixes** — sat in Other, next to 18 `ci(...)` commits, 26 `test(...)`, 6 `feat(...)`, 5 `style(...)`, 4 `specs(...)`, 3 `docs(...)`, 2 `refactor(...)` and 2 `chore(...)`. **Fix:** eight new parser entries, placed AFTER the eight shortcode rules and BEFORE the `.*` catch-all, keyed on `^:[a-z0-9_]+: [(:]` — the type the author declared — plus one shortcode alias, `:test:` → Testing (a typo'd shortcode; no such gitmoji exists, and all 19 uses are tests, including one whose type field is the equally typo'd `render(...)`). Placement is deliberate and narrow: putting the type tier FIRST would re-file large parts of the eight mapped shortcodes too; placing it after means it only sorts what the catch-all was already catching. **Keyed on type, never on the emoji, because the emoji is the unreliable half** — `:lipstick: fix(provider): …` is a fix, not a UI change, and `:art:` is used for both `style(…)` and `refactor(…)`; a mapping by emoji dictionary would have mis-filed both. The inventory was derived from `git log --format=%s` over the FULL history (30 distinct shortcodes, 22 of them unmapped), not from a sample, and **every** unmapped-shortcode subject in this repo turned out to declare a conventional type, which is what makes type-keying total rather than lucky. **Judgment calls, stated because a wrong group is worse than Other:** (a) `specs(…)` → Documentation — spec authoring under `openspec/` is a written artifact, the same class the existing `:memo:|^docs` rule files; (b) `style(…)` → Refactoring — internal code hygiene with no behaviour change, which is what that group already means, and closer than Other; (c) `revert(…)` → **left in Other on purpose**: no existing group fits it and adding a Reverts group is a changelog-structure change, not a categorisation fix; (d) one malformed subject, `:test(release): add CI audit gate for single CodeQL workflow`, declares no parseable type and stays in Other — the commit is published history and cannot be reworded. Those two lines are the entire remaining Other. **Effect on already-published sections, stated as a MULTISET because the raw line diff is misleading:** the rendered bullet multiset is **identical** before and after — 509 unique bullets, 514 rendered lines, nothing added, nothing dropped — and 69 unique bullets move, all in one direction, all out of Other: Testing 28, Chores 20, Documentation 7, Refactoring 7, Features 5, Fixes 2. Zero bullets move between two non-Other groups. This re-files lines inside the already-published `[0.1.0]` and `[0.0.0]` sections; acceptable on the same ground D-136 established and the review accepted — `CHANGELOG.md` is a derived artifact regenerated in full from history, and the published v0.1.0 GitHub Release body is a separate immutable artifact that is unaffected. **Proof, both polarities, in the already-wired `release-changelog-gate-test`:** `hack/release/changelog_gate_test.sh` §8 asserts the `:ambulance:` hotfix renders under Fixes and — structurally, so it keeps holding as history grows — that NO line under Other declares a type this repo files; §8a strips the `# REL-14`-tagged entries (mutation proven to have landed by line count), shows the hotfix falls back to Other, and asserts the rendered subject multiset is unchanged by the re-grouping, so the parsers can only re-file and never add or drop a line. §7a's "changes nothing else" claim was restated over the same multiset in this lane, because removing a group's last member also removes its `###` heading and a raw line diff reads that as unexplained churn. Revert: drop the `# REL-14` entries and §8/§8a — the hotfix returns to Other. | | D-138 | 2026-08-09 | **The three reliability P1s of the 2026-08-09 audit (RELI-01/02/03) are DEFERRED past v0.2.0, deliberately and with the deferral recorded (AGENTS.md rule 6).** All three are **pre-existing at v0.1.0**; nothing in v0.2.0 introduced or worsened any of them, verified against `1d8aa60` (`origin/main` at audit time). Holding the tag would delay two fail-open fixes and a P0 in order to fix defects that are already live. **The machine direction holds in all three:** no probed path merges anything unevaluated — `MergeCAS` re-checks all three pins atomically, arming stays default-deny (`internal/forge/precondition.go`), and the `.assent/**` self-edit guard still yields BLOCK with zero forge writes. What fails is the **human signal channel**, which is why they are P1 and not P0. **RELI-01 — clean exit-0 runs leave a stale APPROVE banner, and D-130's compensating control does NOT apply here.** `cmd/assent/run.go` emits the DecisionRecord at step 8 and only then enters the step-9 switch, whose two GUARD branches (`reservedSelfEditBlock`, `untrustedExecutionContext`) skip `forge.Reconcile` **entirely** — including the summary upsert. So run 1 APPROVEs and posts `✅ Decision: APPROVE`; run 2, after a `.assent/**` edit is added, decides BLOCK, exits **0**, and leaves that note byte-identical. `openspec/specs/p5-e5-provider-host/spec.md` REQ-E5-S08-03 accepts a stale banner on the abort path precisely because "a REVIEW rerun upserts that same summary to REVIEW and adds an unresolved discussion" — the discussion being what holds the merge under `only_allow_merge_if_all_discussions_are_resolved`. **On the GUARD-1 self-edit path no thread is posted at all**, so nothing holds the merge and no later run repairs the banner: the compensating control the spec relies on is absent exactly where it is needed. Reachability is ordinary CI cadence, and it is deliberately triggerable at the cost of two pushes — D-042's own threat model rendering as a green tick. **Its fix is out of release scope BY CONSTRUCTION**, not by preference: `openspec/specs/p5-aud-audit-remediation/spec.md` pins "the decision is BLOCK with zero forge writes (GUARD-1 dominance over the gap-degrade)", so upserting a summary on that path REOPENS a frozen acceptance criterion and needs an openspec change proposal first (see OQ-31). **RELI-02 — a duplicated summary comment is UNRECOVERABLE WITHOUT A HUMAN.** `reconcileThread` has both `repairDuplicates` and a step-9 rescan; `reconcileSummary` (`internal/forge/forge.go`) has neither — it is a bare `UpsertComment`. No branch of `Reconcile` can remove a second summary note, so once two exist the wrong one can sit at `decision=APPROVE` forever. Not a corner case: `PreconditionFromCapabilities` seeds `DuplicatePrevention` to `unserialized-best-effort` because per-MR `resource_group` serialization is never probed, i.e. the race is the project's **declared default posture**. First item of v0.2.1. **RELI-03 — the standing bot approval has no retraction and the cited mitigation is never probed.** `reconcileApproveMerge` records `Approve` before `MergeCAS`; on CAS failure in the TOCTOU window the approval is already written and `forge.Forge` has no `Unapprove` verb. The code's own safety argument named the forge's remove-approvals-on-push setting — and **`reset_approvals_on_push` appears in ZERO Go files**: nothing reads it, `probeCapabilities` does not fetch it, `assent doctor` cannot report it. `spike-secure-setup.md` D11 specified refuse-to-arm when it is off and C19 specified doctor verification; **neither was built**, and the comment's deferral pointer named P4-E1-S10 — a slice that SHIPPED (D-041), so the concern was never picked up. **The comment is corrected in this lane** (`internal/forge/forge.go`, text only, no behaviour change) because an asserted-and-unbuilt mitigation TERMINATES THE SEARCH — it is how this survived two prior audits. RELI-03 needs no open question: D11 already decided it; it is unbuilt, not undecided. **Also recorded here, same shape — the ADR-0019 `duplicate_prevention:` MUST is unmet.** The value is computed and typed through to `PreconditionReport` and then never printed: `emitDoctorReport` emits only the arm verdict and refusal reasons, so no `assent doctor` output contains the string. The safe-default half DOES hold (nothing can claim `single-writer-serialized` without the mechanism). **Judgment call: annotate, do not emit.** Emitting is ~3 lines, but it is one instance of audit **ARCH-11** — doctor computes a typed capability report and prints essentially none of it — so emitting this one field would half-close ARCH-11 and leave the report inconsistent with itself, and it is a user-visible CLI output change, which does not belong in a docs-truth lane before a tag. ADR-0019 now carries the unmet-MUST annotation and points here; the emission belongs to the v0.2.1 ARCH-11 slice with its own tests. **Not deferred silently:** all three plus ARCH-11 are named in the v0.2.1 wave. Revert: `git revert` this lane — restores the false RELI-03 comment and changes no behaviour, since the lane changes none. | | D-139 | 2026-08-09 | **The security lens's SEC-01/SEC-04/SEC-05 are KNOWN and DEFERRED to v0.2.1; the tag proceeds, but the release must not claim `--checkout` is now safe.** All three are pre-existing at v0.1.0 and none was introduced by v0.2.0 (verified at `1d8aa60`). Reach on SEC-01 is bounded and that is why it does not block: **no Go non-test code constructs `--checkout`**, no CI template, example, hack script or e2e passes it, and neither `README.md` nor `docs/usage/walkthrough.md` mentions it at all — the adoption path this project actually teaches is checkout-less. The fix is also monotone-safer to ship after the tag, since the P0 already merged in this release was likewise `--checkout`-only. **SEC-01 — the `--checkout` tree is never bound to the evaluated SHA.** With `--checkout` the local tree is the sole authority for the judged bytes and the changed-file set (D-077), while `pins.sourceSha`/`targetSha` and the compare-and-swap come from the forge; **nothing compares the two**. Measured: `cmd/assent/checkout.go` contains zero occurrences of `SHA`/`Sha`, and `run.go` has no step relating the tree to a commit. Reproduced by the lens: forge head a destructive shrink, local checkout a benign grow → `APPROVE`, `approvals=1 merges=1`. ADR-0015 §2 promises every write re-verifies what it acts on; the guard re-verifies that the *metadata* has not moved, never that the judged *bytes* are the bytes at that SHA. **Condition attached and DISCHARGED IN THIS LANE:** this release hardens `--checkout` extensively (D-133 refuses any symlink anywhere) and `docs/usage/cli.md` already named "run without `--checkout`" as remedy #1, so the page read as though the flag had been made sound. A *Known limitation: the checkout is not bound to the evaluated commit* section now says otherwise, framed as a property of how the checkout is CONSTRUCTED (build `head/` from the MR head SHA; cancel superseded pipelines) and **claiming no mitigation on assent's part** — `auto_cancel_redundant_pipelines` is a project setting the tool never probes, and citing an unread setting as a control is the exact pattern this audit found three times. **Named fix for v0.2.1:** bind the checkout to the evaluated SHA, or cross-check the local changed-file set against the already-fetched `snapshot.ChangedFiles` and fold divergence opaque. **SEC-04 — protected-config verification is a substring test.** `internal/forge/gitlab/snapshot.go` sets `caps.ProtectedPipelineExternal = strings.Contains(proj.CIConfigPath, "@")`, while ADR-0015 §4 calls protected config THE load-bearing adoption prerequisite and says doctor refuses to arm when it cannot verify it. Reproduced: an in-repo, author-editable `.ci/pipeline@v2.yml` arms and merges. **Named fix:** replace the substring test with a structural parse — require a non-empty suffix after `@` that contains `/`. **SEC-05 — absent provenance is read as trusted.** `snapshot.go` computes `ForkMR: mrResp.SourceProjectID != 0 && mrResp.SourceProjectID != mrResp.ProjectID`, so an absent or zero `source_project_id` yields `ForkMR=false` and the fork guard never engages; reproduced to `approvals=1 merges=1`. **Named fix:** treat absent or zero `source_project_id` as **fork** (untrusted) — unknown provenance must fail toward advisory-only. **The composition worth not losing, recorded AS A HYPOTHESIS, not as measured:** SEC-05 × SEC-03 would let a fork MR reach the write path and then leave a **standing bot approval on a fork MR**, converting ADR-0015 §8's "CI on fork / untrusted-contributor MR → advisory-only, no writes" into "we can leave an approval on a fork". There is **no evidence real GitLab ever omits `source_project_id`**; the composition is untested and is written down so the v0.2.1 SEC-05 fix is understood as closing more than a provenance nicety. **Correction carried from the lens:** its SEC-08 was **WITHDRAWN as non-novel** — D-130 already documents the host-declaration `continue`-on-any-error as a knowingly-deferred sibling. It is not a new finding and must not be written up as one. | -| D-140 | 2026-08-10 | **E10 (GitHub adapter + Actions entrypoint) is UNLOCKED by direct operator instruction; ADR-0021 governs the seam and `openspec/specs/p5-e10-github-forge/spec.md` decomposes it into 19 stories (S00, the addressing & representation model, was added after an adversarial review found two P0 representation defects).** E10 was **Locked** under D-012 ("unlocks with a named consumer"), and that lock was reaffirmed twice — D-017 named the consumer and still said "E10/E13 stay locked", D-019 restated "GitHub + remote packs locked". No prior decision authorized it, so the operator's instruction this session is a NEW unlock event, recorded here BEFORE any spec text or code exists (AGENTS.md rule 6). **What this authorizes**: the GitHub forge adapter, the multi-adapter seam work it depends on, and forge selection in `run`/`doctor`. **What it does NOT authorize**: E13 remote packs (still Locked under D-012 — the "named consumer" reasoning is untouched for that tier), E12 `serve`, E14 CRD, or any third forge / plugin-forge protocol (ADR-0021 Option D, rejected — no named consumer). **Scope of v1 GitHub**: behavioural parity for the GATE, not device-for-device parity (dossier §3, OQ-7/OQ-18 — required-conversation-resolution carries acknowledgement, `REQUEST_CHANGES` reserved for block); the three known deltas (review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, and where GitHub cannot prove what the gate needs the adapter reports the gap and NEVER arms — the same shape as GitLab free tier. **Why a seam epic and not just an adapter**: the 2026-08-09 audit's ARCH-18/ARCH-19 found `docs/planning/design-notes/e10-forge-port-lift.md` under-scopes the epic, and that the conformance suite cannot be run by a second adapter because all ~1,166 lines live in `_test.go` files Go cannot import — so a GitHub adapter written today would be developed against no executable contract and D-084's `github-deferred` catalog rows would be unflippable by construction. The original ARCH-18/ARCH-19 finding text is NOT in the repo (only the one-line summary at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives); the two design buckets were therefore RE-DERIVED as (A) no neutral capability model — the GitHub dossier §4 names eleven flags the port needs while `probeCapabilities` reads three project fields and `capabilityGap` is computed in GitLab terms — and (B) no port-level transport/auth policy — GitHub thread resolution is GraphQL-only and needs PAT-vs-App auth, while bounded reads/pagination caps (AUD-S10) and idempotent-GET retry/deadlines (AUD-S11) live inside the GitLab client. Both are recorded as a re-derivation, not as a citation. **OPEN SUB-QUESTION (operator)**: whether the Actions entrypoint (E10-S16) stays in scope — `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", but the action is packaging on top of an adapter and is the one story whose absence leaves everything else useful; default is to keep it last and independently droppable. **Consequence to watch (E10-S04)**: forcing every capability to be stated explicitly may turn a GitLab arming path that passes today into an honest capability gap — a user-visible behaviour change that gets its OWN decision row and changelog entry, never absorbed silently into "E10 refactor". Revert: re-lock E10 in `later-phases.md`, delete the spec directory; nothing here is published API (`forge.RunPort` is `internal/`), so no compatibility window applies. | -| D-141 | 2026-08-10 | **E11 (Rego complex-rule backend) implementation is UNLOCKED by direct operator instruction; `openspec/specs/p5-e11-rego-backend/spec.md` decomposes it into 13 stories under ADR-0002 v2.** E11's CONTRACT was already unlocked by D-017; what was gated was IMPLEMENTATION, twice: "after Phase 4" (satisfied — the Phase-4 adoption gate closed with D-042) and, per D-017, **evidence-based per rule** ("each ported rule tries CEL first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling"). This row records the operator lifting that per-rule evidence gate. **What it does NOT waive**: the DESIGN need the gate was protecting — E11-S01 still requires a written tier-1 ceiling document with concrete rules per shape (multi-pass, cross-manifest, set-difference, graph-relationship), and any shape found CEL-expressible is struck from scope. **What it does NOT authorize**: WASM or gRPC predicate backends (still Locked under D-012 — this unlocks Rego only); domain-aware joins and in-process Go rule plugins (D-017 DECLINED both permanently — not deferred); giving Rego any control over aggregation, effects, or points (ADR-0002 v2 boundary); any `EvaluationInput` change. **Two constraints found during design that shape the epic**: (1) **E11 is the first epic whose DoD is `git diff schemas/` != 0.** P3-E1-S02's backend-neutrality guarantee ("no field naming a predicate backend anywhere in the schema") applies to `EvaluationInput` and HOLDS — no decision contract changes — but `schemas/policy/v1alpha1/merge-policy.schema.json` defines the predicate leaf as `additionalProperties:false, required:["cel"]`, so a `rego:` leaf IS a policy-schema change. `API_STABILITY.md:19` permits exactly this within `v1alpha1` as an announced additive change with an openspec change and no `apiVersion` bump; the change is backward-compatible and deliberately forward-INcompatible (an older binary rejects a `rego:` leaf by strict-decode, which is the correct direction — it must not silently ignore a rule it cannot evaluate). A reviewer applying the previous epics' `git diff schemas/ == 0` habit will flag the correct change as a violation; E11-S02-04 scopes the drift guard rather than deleting it. (2) **The reflexive safety measure violates AGENTS.md rule 7.** Rego ships `time.now_ns()`, `rand.intn()`, and `http.send()`, all of which are denied structurally by the D-013 capability sandbox (E11-S04, compile-time failure, golden allowlist so an OPA upgrade cannot widen it unnoticed) — but bounding evaluation with a WALL-CLOCK TIMEOUT would itself be a rule-7 violation, making the same policy over the same ChangeSet decide differently on a slow runner. E11-S06 therefore requires a machine-independent evaluation budget, and exceeding it must be a PROCESS ERROR that can never be a policy outcome; "timeout → BLOCK" is explicitly rejected as machine-dependent while merely LOOKING fail-closed. **Also fixed by the spec**: zero violations NEVER proves a required obligation (`later-phases.md`'s explicit polarity rule; E11-S07-02 tests the failing polarity). **🔴 BLOCKING OPEN SUB-QUESTION (operator) — adopting OPA narrows rule 7's MECHANISM, and both existing purity gates would miss it.** Verified during the design session: `internal/core/purity_test.go` flags only each guarded file's OWN imports (`math/rand`, `crypto/rand`, `net`, `net/*`) and selectors (`os.Getenv`, `time.Now`), and `.golangci.yml`'s `pure-tree` depguard is `list-mode: lax`, deny-only, over DIRECT imports — **neither is transitive**. A file in `internal/core/**` importing `github.com/open-policy-agent/opa/rego` therefore passes both gates GREEN while transitively linking `net/http` (OPA ships the `http.send` builtin), defeating the `net` deny that encodes D-123 / rule 7 invisibly. S04's capability sandbox makes `http.send` uncallable FROM POLICY — the real threat — but the guarantee's nature changes from "the network stack is not linked into the decision path" (structural, greppable) to "linked but unreachable from policy" (behavioural, resting on a capability file). That is a hard-rule change and cannot be made by a story. Options: **(d1)** accept the narrowing with an ADR-0011/rule-7 amendment plus a transitive `go list -deps` purity check allowlisting exactly the OPA path (RECOMMENDED); **(d2)** keep the guarded tree OPA-free by injecting an evaluator from `cmd/assent` — honest, but it moves part of the decision path outside the tree rule 7 guards; **(d3)** drop OPA (a hand-rolled evaluator would be far worse; rejected unless d1 and d2 are). **E11-S04 is blocked on this answer** (it decides the evaluator's package and its gate); S01–S03 are unblocked. Separately on supply chain: OPA is a large dependency with a large transitive tree on a project shipping cosign/SLSA provenance, `govulncheck`, and Scorecard — recommended default is accept-and-pin, with S03 recording the binary-size delta. Revert: re-assert the D-017 per-rule evidence gate, delete the spec directory, leave the `# locked: D-012` quarantine marker in place. | +| D-140 | 2026-08-10 | **E10 (GitHub adapter + Actions entrypoint) is UNLOCKED by direct operator instruction; ADR-0021 governs the seam and `openspec/specs/p5-e10-github-forge/spec.md` decomposes it into 19 stories (S00, the addressing & representation model, was added after an adversarial review found two P0 representation defects).** E10 was **Locked** under D-012 ("unlocks with a named consumer"), and that lock was reaffirmed twice — D-017 named the consumer and still said "E10/E13 stay locked", D-019 restated "GitHub + remote packs locked". No prior decision authorized it, so the operator's instruction this session is a NEW unlock event, recorded here BEFORE any spec text or code exists (AGENTS.md rule 6). **What this authorizes**: the GitHub forge adapter, the multi-adapter seam work it depends on, and forge selection in `run`/`doctor`. **What it does NOT authorize**: E13 remote packs (still Locked under D-012 — the "named consumer" reasoning is untouched for that tier), E12 `serve`, E14 CRD, or any third forge / plugin-forge protocol (ADR-0021 Option D, rejected — no named consumer). **Scope of v1 GitHub**: behavioural parity for the GATE, not device-for-device parity (dossier §3, OQ-7/OQ-18 — required-conversation-resolution carries acknowledgement, `REQUEST_CHANGES` reserved for block); the three known deltas (review dismissal, auto-merge revoke, merge queue) are modelled as capabilities, and where GitHub cannot prove what the gate needs the adapter reports the gap and NEVER arms — the same shape as GitLab free tier. **Why a seam epic and not just an adapter**: the 2026-08-09 audit's ARCH-18/ARCH-19 found `docs/planning/design-notes/e10-forge-port-lift.md` under-scopes the epic, and that the conformance suite cannot be run by a second adapter because all 1,155 lines live in `_test.go` files Go cannot import (the package totals 1,166 including the non-test `doc.go`; the `~1,166` figure in the source design note was corrected on 2026-08-10) — so a GitHub adapter written today would be developed against no executable contract and D-084's `github-deferred` catalog rows would be unflippable by construction. The original ARCH-18/ARCH-19 finding text is NOT in the repo (only the one-line summary at `agent-context/PROJECT-AUDIT-2026-08-09.md:412` survives); the two design buckets were therefore RE-DERIVED as (A) no neutral capability model — the GitHub dossier §4 names eleven flags the port needs while `probeCapabilities` reads three project fields and `capabilityGap` is computed in GitLab terms — and (B) no port-level transport/auth policy — GitHub thread resolution is GraphQL-only and needs PAT-vs-App auth, while bounded reads/pagination caps (AUD-S10) and idempotent-GET retry/deadlines (AUD-S11) live inside the GitLab client. Both are recorded as a re-derivation, not as a citation. **OPEN SUB-QUESTION (operator)**: whether the Actions entrypoint (E10-S16) stays in scope — `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", but the action is packaging on top of an adapter and is the one story whose absence leaves everything else useful; default is to keep it last and independently droppable. **Consequence to watch (E10-S04)**: forcing every capability to be stated explicitly may turn a GitLab arming path that passes today into an honest capability gap — a user-visible behaviour change that gets its OWN decision row and changelog entry, never absorbed silently into "E10 refactor". Revert: re-lock E10 in `later-phases.md`, delete the spec directory; nothing here is published API (`forge.RunPort` is `internal/`), so no compatibility window applies. **✅ OPEN SUB-QUESTION CLOSED (operator, 2026-08-10): option (a) — the Actions entrypoint (E10-S16) STAYS in E10's scope**, kept last and independently droppable; `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", so cutting it would have needed the plan text corrected. E10-S16 is no longer operator-gated. | +| D-141 | 2026-08-10 | **E11 (Rego complex-rule backend) implementation is UNLOCKED by direct operator instruction; `openspec/specs/p5-e11-rego-backend/spec.md` decomposes it into 13 stories under ADR-0002 v2.** E11's CONTRACT was already unlocked by D-017; what was gated was IMPLEMENTATION, twice: "after Phase 4" (satisfied — the Phase-4 adoption gate closed with D-042) and, per D-017, **evidence-based per rule** ("each ported rule tries CEL first, the backend is built when a concrete rule demonstrably exceeds the tier-1 ceiling"). This row records the operator lifting that per-rule evidence gate. **What it does NOT waive**: the DESIGN need the gate was protecting — E11-S01 still requires a written tier-1 ceiling document with concrete rules per shape (multi-pass, cross-manifest, set-difference, graph-relationship), and any shape found CEL-expressible is struck from scope. **What it does NOT authorize**: WASM or gRPC predicate backends (still Locked under D-012 — this unlocks Rego only); domain-aware joins and in-process Go rule plugins (D-017 DECLINED both permanently — not deferred); giving Rego any control over aggregation, effects, or points (ADR-0002 v2 boundary); any `EvaluationInput` change. **Two constraints found during design that shape the epic**: (1) **E11 is the first epic whose DoD is `git diff schemas/` != 0.** P3-E1-S02's backend-neutrality guarantee ("no field naming a predicate backend anywhere in the schema") applies to `EvaluationInput` and HOLDS — no decision contract changes — but `schemas/policy/v1alpha1/merge-policy.schema.json` defines the predicate leaf as `additionalProperties:false, required:["cel"]`, so a `rego:` leaf IS a policy-schema change. `API_STABILITY.md:19` permits exactly this within `v1alpha1` as an announced additive change with an openspec change and no `apiVersion` bump; the change is backward-compatible and deliberately forward-INcompatible (an older binary rejects a `rego:` leaf by strict-decode, which is the correct direction — it must not silently ignore a rule it cannot evaluate). A reviewer applying the previous epics' `git diff schemas/ == 0` habit will flag the correct change as a violation; E11-S02-04 scopes the drift guard rather than deleting it. (2) **The reflexive safety measure violates AGENTS.md rule 7.** Rego ships `time.now_ns()`, `rand.intn()`, and `http.send()`, all of which are denied structurally by the D-013 capability sandbox (E11-S04, compile-time failure, golden allowlist so an OPA upgrade cannot widen it unnoticed) — but bounding evaluation with a WALL-CLOCK TIMEOUT would itself be a rule-7 violation, making the same policy over the same ChangeSet decide differently on a slow runner. E11-S06 therefore requires a machine-independent evaluation budget, and exceeding it must be a PROCESS ERROR that can never be a policy outcome; "timeout → BLOCK" is explicitly rejected as machine-dependent while merely LOOKING fail-closed. **Also fixed by the spec**: zero violations NEVER proves a required obligation (`later-phases.md`'s explicit polarity rule; E11-S07-02 tests the failing polarity). **🔴 BLOCKING OPEN SUB-QUESTION (operator) — adopting OPA narrows rule 7's MECHANISM, and both existing purity gates would miss it.** Verified during the design session: `internal/core/purity_test.go` flags only each guarded file's OWN imports (`math/rand`, `crypto/rand`, `net`, `net/*`) and selectors (`os.Getenv`, `time.Now`), and `.golangci.yml`'s `pure-tree` depguard is `list-mode: lax`, deny-only, over DIRECT imports — **neither is transitive**. A file in `internal/core/**` importing `github.com/open-policy-agent/opa/rego` therefore passes both gates GREEN while transitively linking `net/http` (OPA ships the `http.send` builtin), defeating the `net` deny that encodes D-123 / rule 7 invisibly. S04's capability sandbox makes `http.send` uncallable FROM POLICY — the real threat — but the guarantee's nature changes from "the network stack is not linked into the decision path" (structural, greppable) to "linked but unreachable from policy" (behavioural, resting on a capability file). That is a hard-rule change and cannot be made by a story. Options: **(d1)** accept the narrowing with an ADR-0011/rule-7 amendment plus a transitive `go list -deps` purity check allowlisting exactly the OPA path (RECOMMENDED); **(d2)** keep the guarded tree OPA-free by injecting an evaluator from `cmd/assent` — honest, but it moves part of the decision path outside the tree rule 7 guards; **(d3)** drop OPA (a hand-rolled evaluator would be far worse; rejected unless d1 and d2 are). **E11-S04 is blocked on this answer** (it decides the evaluator's package and its gate); S01–S03 are unblocked. Separately on supply chain: OPA is a large dependency with a large transitive tree on a project shipping cosign/SLSA provenance, `govulncheck`, and Scorecard — recommended default is accept-and-pin, with S03 recording the binary-size delta. Revert: re-assert the D-017 per-rule evidence gate, delete the spec directory, leave the `# locked: D-012` quarantine marker in place. **✅ PARTIAL ANSWER (operator, 2026-08-10): `github.com/open-policy-agent/opa` is ACCEPTED as a dependency and pinned** — this closes the *supply-chain* half of judgment call (d) and rejects **(d3) drop OPA**. **🔴 The MECHANISM half is still OPEN and still blocks E11-S03 and E11-S04: (d1) accept the rule-7 narrowing explicitly (capability-enforced, plus a transitive purity guard) vs (d2) keep the guarded tree OPA-free behind an injected interface.** These are not the same question and "accept and pin" does not settle it — **(d2) also accepts and pins OPA**; it differs on *where the evaluator lives* and *which gate enforces rule 7*. Reading the dependency answer as settling the mechanism would silently choose (d1), i.e. narrow a hard rule by merging a story, which rule 6 forbids. Whichever way it resolves needs an **ADR-0011/rule-7 amendment plus its own D-row** (REQ-E11-S04-04). **Also corrected here (2026-08-10, independent review of PR #45):** the spec originally said E11-S01–S03 were unblocked while (d) was pending. **False — E11-S03 is the story that *effects* the narrowing** (REQ-E11-S03-03 adds OPA to `go.mod`; its Test paths sit in the D-123 guarded tree), and because *neither* purity gate is transitive it would have landed green. S03 is now explicitly blocked on (d). The epic also gains **E11-S00**, a deterministic-budget feasibility spike built in a **nested throwaway module** (no `go.work` exists, so Go excludes it from the root module's `./...`) so the question can be answered without adopting the dependency (d) has not authorised; E11 is therefore **14 stories, S00–S13**, and E11-S02 is blocked on S00 because an observe-only `rego:` leaf is a different published contract from a gating one. | diff --git a/openspec/specs/backlog.md b/openspec/specs/backlog.md index f85b9a20..bb91a593 100644 --- a/openspec/specs/backlog.md +++ b/openspec/specs/backlog.md @@ -575,7 +575,7 @@ them rather than auto-merge. | E10-S13 | Forge selection in `run`/`doctor`; ambiguity fails closed | **[autonomous]** | S12 | no default-to-GitLab | | E10-S14 | Conformance parity — **every** row needs an adapter disposition, not just deferrals | **[autonomous]** | S13 | else GitHub ships with 0 trust-boundary cases proven | | E10-S15 | Docs & maturity truth (README tier, C4, `--forge`, dossier items) | **[autonomous]** | S14 | no doc claims an `unknown` capability | -| E10-S16 | Actions entrypoint (`action.yml`, pinned binary, base-ref trust) | **[autonomous — scope-flagged]** | S15 | **D-140 open sub-question**; independently droppable | +| E10-S16 | Actions entrypoint (`action.yml`, pinned binary, base-ref trust) | **[autonomous]** | S15 | ✅ **operator-answered 2026-08-10: stays in E10**, last + independently droppable | | E10-S17 | Exit gate | **[autonomous]** | S01–S16 | **the E10 exit gate** | | E10-S18 | Live GitHub adoption proof on a real repo (mirrors D-042) | **[infra-gated · operator]** | S17 + infra | D-012-grade evidence; not an autonomous blocker | @@ -590,11 +590,11 @@ LGTM** (published contract + the decision path itself). Independent of E10; may | ID | Story | Execution | Depends on | Gate contribution | | --- | --- | --- | --- | --- | +| E11-S00 | ⚠️ **SPIKE, do first**: does OPA expose a deterministic (non-wall-clock) eval budget? Nested throwaway module — root `go.mod` unchanged | **[autonomous · spike]** | none | if not, S06 stalls the epic *after* S02+S03 commit | | E11-S01 | Record the tier-1 (CEL) ceiling with concrete exceeding rules | **[autonomous]** | none | **do first** — a CEL-expressible shape is struck from scope | -| E11-S06′ | ⚠️ **SPIKE first**: does OPA expose a deterministic (non-wall-clock) eval budget? | **[autonomous · spike]** | S01 | if not, S06 stalls the epic *after* S02+S03 commit | -| E11-S02 | ⚠️ Additive `rego:` leaf in the policy schema (announced, no `apiVersion` bump) | **[autonomous · engine-grade · LGTM]** | S01, S06′ | drift guard scoped; both polarities tested | -| E11-S03 | Module loading from the **target ref**; compile failure is a lint hard error | **[autonomous]** | S02 | no second, laxer load path | -| E11-S04 | 🔴 OPA capability sandbox — **blocked on the operator's rule-7 answer (d1/d2/d3)** | **[autonomous · engine-grade · LGTM]** | S03 + operator | both purity gates are non-transitive; see D-141 | +| E11-S02 | ⚠️ Additive `rego:` leaf in the policy schema (announced, no `apiVersion` bump) | **[autonomous · engine-grade · LGTM]** | **S00**, S01 | drift guard scoped; both polarities tested | +| E11-S03 | 🔴 Module loading from the **target ref**; compile failure is a lint hard error — **blocked on the operator's rule-7 answer (d1/d2)**: this story adds OPA to `go.mod` inside the guarded tree | **[autonomous · engine-grade · LGTM]** | S02 + operator | no second, laxer load path; transitive purity guard under (d1) | +| E11-S04 | 🔴 OPA capability sandbox — **blocked on the operator's rule-7 *mechanism* answer (d1 vs d2)**; "accept and pin" settled only the supply-chain half | **[autonomous · engine-grade · LGTM]** | S03 + operator | both purity gates are non-transitive; see D-141 | | E11-S05 | ⚠️ Input binding to the identical `EvaluationInput` | **[autonomous · engine-grade]** | S04 | proves P3-E1-S02 neutrality empirically | | E11-S06 | ⚠️ Deterministic evaluation budget (never wall-clock) | **[autonomous · engine-grade · LGTM]** | S05 | N≥100 identical runs; budget ≠ decision | | E11-S07 | ⚠️ Violations → findings; **zero violations never proves an obligation** | **[autonomous · engine-grade · LGTM]** | S06 | the failing polarity is tested | @@ -603,7 +603,7 @@ LGTM** (published contract + the decision path itself). Independent of E10; may | E11-S10 | `assent test` support + both-polarity coverage | **[autonomous]** | S08 | ADR-0014 unchanged | | E11-S11 | Remove the `# locked: D-012` quarantine; **update** the P3-E3-S04 guard | **[autonomous]** | S10 | only E11's lane may do this | | E11-S12 | Docs & maturity truth; retire ADR-0002's "pluggable half unbuilt" line | **[autonomous]** | S11 | nothing still calls Rego locked | -| E11-S13 | Exit gate | **[autonomous]** | S01–S12 | **the E11 exit gate** | +| E11-S13 | Exit gate | **[autonomous]** | S00–S12 | **the E11 exit gate** | ## Phases 3–5 @@ -614,7 +614,7 @@ Epic paragraphs (goal, ADR constraints, exit gate, story seeds) in | --- | --- | --- | | 3 — Contracts first | P3-E1 schemas + contract fixture (incl. ApprovalEvidence + named-consumer fixture) · P3-E2 versioning/compat spec · P3-E3 example migration · P3-E4 lifecycle: phase/profiles/comparison (ADR-0018) · P3-E5 publication reconciliation protocol (ADR-0019) | strict end-to-end contract fixture validates (ADR-0017 §8, D-016); new ADRs 0018/0019 accepted at the freeze review | | 4 — Walking skeleton | P4-E1 (+ rerun-idempotence gate, D-017) · **P2-E4-NS (OQ-24 timed run)** · holdout adjudication (OQ-25) | L3 skeleton green + **one real repo on live MRs** (D-012); north-star wording only after timed run | -| 5 — Implementation | E1–E7 **DONE**; **E7 AUTONOMOUS COMPLETE** (S01–S05+S08, D-087); **E8 AUTONOMOUS COMPLETE** ([p5-e8-renderer/spec.md](p5-e8-renderer/spec.md), S01–S14, D-098); **E9 AUTONOMOUS COMPLETE** ([p5-e9-distribution/spec.md](p5-e9-distribution/spec.md), S01–S13, D-099–D-111 CLOSED; Homebrew Formula live; PAT rotate optional); **PCS AUTONOMOUS COMPLETE** ([p5-pcs-policy-comparison/spec.md](p5-pcs-policy-comparison/spec.md), S01–S09, **D-057 closed**, D-118); **E10 UNLOCKED + DECOMPOSED** (D-140, [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md), 19 stories, ADR-0021); **E11 IMPLEMENTATION UNLOCKED + DECOMPOSED** (D-141, [p5-e11-rego-backend/spec.md](p5-e11-rego-backend/spec.md), 13 stories); E12 **contract-unlocked** (D-017), not decomposed; E14 gated on Spike D; **E13 still locked** (D-012) | per-epic; E9 exit = tagged signed release + docs live + brew Formula (D-111); PAT rotate optional | +| 5 — Implementation | E1–E7 **DONE**; **E7 AUTONOMOUS COMPLETE** (S01–S05+S08, D-087); **E8 AUTONOMOUS COMPLETE** ([p5-e8-renderer/spec.md](p5-e8-renderer/spec.md), S01–S14, D-098); **E9 AUTONOMOUS COMPLETE** ([p5-e9-distribution/spec.md](p5-e9-distribution/spec.md), S01–S13, D-099–D-111 CLOSED; Homebrew Formula live; PAT rotate optional); **PCS AUTONOMOUS COMPLETE** ([p5-pcs-policy-comparison/spec.md](p5-pcs-policy-comparison/spec.md), S01–S09, **D-057 closed**, D-118); **E10 UNLOCKED + DECOMPOSED** (D-140, [p5-e10-github-forge/spec.md](p5-e10-github-forge/spec.md), 19 stories, ADR-0021); **E11 IMPLEMENTATION UNLOCKED + DECOMPOSED** (D-141, [p5-e11-rego-backend/spec.md](p5-e11-rego-backend/spec.md), 14 stories); E12 **contract-unlocked** (D-017), not decomposed; E14 gated on Spike D; **E13 still locked** (D-012) | per-epic; E9 exit = tagged signed release + docs live + brew Formula (D-111); PAT rotate optional | Named-consumer disposition (what unlocked, what stayed locked, and why): [docs/planning/named-consumer-compat.md](../../docs/planning/named-consumer-compat.md). diff --git a/openspec/specs/later-phases.md b/openspec/specs/later-phases.md index d4c70da3..d49f7b02 100644 --- a/openspec/specs/later-phases.md +++ b/openspec/specs/later-phases.md @@ -264,7 +264,7 @@ dossier (REQUEST_CHANGES + conversation-resolution parity, merge queue as merge- base-ref workflow trust) and by the conformance suite being forge-neutral (E7). **19 stories (S00–S17 autonomous, S18 infra-gated)** governed by **ADR-0021**, which decides the three things the audit found under-scoped (ARCH-18/ARCH-19): the named `forge.RunPort` -composite port, an **importable** conformance suite (today all ~1,166 lines are in `_test.go` +composite port, an **importable** conformance suite (today all 1,155 lines are in `_test.go` files a second adapter cannot import), a neutral capability model where `unknown` never arms, and port-level transport/auth policy. Ordering is normative: the seam (S01–S05) lands before the first GitHub API call. v1 target is behavioural parity for the **gate**, with capability @@ -274,8 +274,8 @@ gaps failing closed. Contract unlocked by D-017; implementation was gated twice — "after Phase 4" (satisfied, the adoption gate closed with D-042) and D-017's **per-rule evidence** gate, which D-141 lifts by operator instruction. The design need survives the lift: E11-S01 still writes the tier-1 -ceiling document, and any shape found CEL-expressible is struck from scope. **13 stories, all -autonomous.** Two constraints found during design: E11 is the **first epic whose DoD is +ceiling document, and any shape found CEL-expressible is struck from scope. **14 stories +(S00–S13), all autonomous.** Two constraints found during design: E11 is the **first epic whose DoD is `git diff schemas/` != 0** (the `rego:` leaf is an announced additive change to `merge-policy.schema.json` per `API_STABILITY.md`; `EvaluationInput` is untouched, so P3-E1-S02's neutrality guarantee holds), and bounding evaluation with a **wall-clock timeout diff --git a/openspec/specs/p5-e10-github-forge/spec.md b/openspec/specs/p5-e10-github-forge/spec.md index 8df41f05..0085b41c 100644 --- a/openspec/specs/p5-e10-github-forge/spec.md +++ b/openspec/specs/p5-e10-github-forge/spec.md @@ -12,7 +12,7 @@ half-built. AUD-S15 lifted `MRInfo`/`ErrNotFound` into `internal/forge/port.go`, `cmd/assent`'s `forgePort` is still an anonymous interface literal at the call site, `run.go` still calls `gitlab.SyntheticDigest`, capability vocabulary is GitLab-private, transport policy (bounded reads, pagination caps, retry, deadlines) lives inside -`internal/forge/gitlab`, and **all 1,166 lines of `internal/forge/conformance` are in +`internal/forge/gitlab`, and **all 1,155 lines of `internal/forge/conformance` are in `_test.go` files Go cannot import** — so the suite that defines "behaves like a forge" cannot be run by a second adapter. The 2026-08-09 audit recorded this as ARCH-18/ARCH-19: `e10-forge-port-lift.md` under-scopes the epic by two design buckets (capability model; @@ -90,8 +90,9 @@ fixtures, P1-E3-S03 GitHub dossier, AUD-S10/S11 transport hardening, AUD-S15 por **New**: importable conformance runner, `forge.RunPort`, `forge.Capability`/`CapabilityReport`, `internal/forge/github`, forge selection, `action.yml`. -**Executability**: S01–S17 **`[autonomous]`** with httptest servers (REST **and** GraphQL) and -the in-memory fake. S02/S04 additionally **`[maintainer LGTM]`** — ADR-0021 names the port and +**Executability**: S00–S17 **`[autonomous]`** with httptest servers (REST **and** GraphQL) and +the in-memory fake. S00 is a **design** story (a document, no code) and S00/S02/S04 additionally +**`[maintainer LGTM]`** — ADR-0021 names the port and capability model core-contract work per GOVERNANCE, and `/agent-loop-auto`'s stop conditions already require surfacing public-API/core-contract changes rather than auto-merging them. S18 **`[infra-gated · operator]`** (a real GitHub repository, live PRs, a token). @@ -106,13 +107,14 @@ executable contract. ## Judgment calls (decide-and-log / operator) -(a) **🟡 OPERATOR — Actions entrypoint (S16) is IN scope but LAST and independently -droppable.** `later-phases.md` titles the epic "GitHub adapter + Actions entrypoint", so -dropping it silently would contradict the plan; but a composite action is a *distribution* -concern (E9's domain) sitting on top of an adapter, and it is the one story whose absence -leaves everything else useful. Recommended default: keep S16 as the final pre-gate story; -if the operator prefers, cut it to a follow-on and the exit gate (S17) drops its row without -any other change. **Recorded as D-140's open sub-question.** +(a) **✅ OPERATOR-ANSWERED (2026-08-10): option (a) — Actions entrypoint (S16) is IN scope but +LAST and independently droppable.** `later-phases.md` titles the epic "GitHub adapter + Actions +entrypoint", so dropping it silently would contradict the plan; but a composite action is a +*distribution* concern (E9's domain) sitting on top of an adapter, and it is the one story whose +absence leaves everything else useful. The operator confirmed the recommended default: **keep +S16 as the final pre-gate story**; should it later be cut to a follow-on, the exit gate (S17) +drops its row without any other change. This sub-question of D-140 is **closed** — S16 is not +blocked and needs no further operator input. (b) **DECIDED — v1 GitHub target is behavioural parity for the *gate*, with capability gaps failing closed.** Per dossier §3 and OQ-7/OQ-18: required-conversation-resolution carries @@ -200,7 +202,8 @@ runs it. - **Goal**: a second adapter can execute the *existing* forge conformance cases without copying them. -- **Why now**: `internal/forge/conformance` is 1,166 lines across four `_test.go` files. Go +- **Why now**: `internal/forge/conformance` is 1,155 lines across four `_test.go` files (the + package totals 1,166 including the 11-line non-test `doc.go`). Go cannot import `_test.go`, so today the only way to conformance-test a new adapter is duplication — which guarantees drift and makes D-084's `github-deferred` rows unflippable. - **Dependencies**: S00. @@ -264,8 +267,14 @@ runs it. - **REQ-E10-S02-01** — Given ADR-0021 §1, when `forge.RunPort` is declared, then it composes `forge.Forge`, `forge.Snapshotter`, `forge.Resolver`, `Describe(project, mr string) - (forge.MRInfo, error)` and `FileAtRef(project, path, ref string) ([]byte, error)`, and - `cmd/assent` references that named type only. + (forge.MRInfo, error)`, `FileAtRef(project, path, ref string) ([]byte, error)` **and** + `FileAtBase(mr, path string) ([]byte, error)` / `FileAtHead(mr, path string) ([]byte, error)`, + and `cmd/assent` references that named type only. **Both accessors are required and they are + not interchangeable** — REQ-E10-S02-05 binds which is legal where. `FileAtRef` is retained + **only** for the ref-addressed *policy* loads ADR-0015 §1 mandates (`cmd/assent/run.go:203`, + `:211`, `:253` — MergePolicy, RulesetBinding, pack, all from the target ref by name); + implementing this REQ by freezing `FileAtRef` as the *sole* content accessor satisfies the + signature while preserving the fabricated-DELETE defect, and is a failure of this story. - Test: `internal/forge/port.go`, `cmd/assent/run.go` - Verify: `go build ./... && go test ./cmd/... ./internal/forge/...` - Level: L1 @@ -291,14 +300,25 @@ runs it. - Test: `hack/lint/depguard_test.sh` - Verify: `task lint` - Level: L1 -- **REQ-E10-S02-05** — Given ADR-0021 item 5, when `RunPort` is declared, then content is - addressed **relative to the merge request** (`FileAtBase(mr, path)` / `FileAtHead(mr, - path)`), not by `(project, branch-name)`, so an adapter owns how it reaches a fork's head. - A conformance case proves a **fork MR with an unchanged governed file yields NO lifecycle - event** on every adapter — the fabricated-DELETE defect. Smuggling `refs/pull/N/head` into - `MRInfo.SourceBranch` is rejected: it corrupts a documented field and leaks into rendering. +- **REQ-E10-S02-05** — Given ADR-0021 item 5, when `RunPort` is declared, then **the governed + subject** is addressed **relative to the merge request** (`FileAtBase(mr, path)` / + `FileAtHead(mr, path)`), not by `(project, branch-name)`, so an adapter owns how it reaches a + fork's head. A conformance case proves a **fork MR with an unchanged governed file yields NO + lifecycle event** on every adapter — the fabricated-DELETE defect. Smuggling + `refs/pull/N/head` into `MRInfo.SourceBranch` is rejected: it corrupts a documented field and + leaks into rendering. + **The boundary is enforced in both directions, and both are asserted:** + (i) the governed-subject reads (`cmd/assent/run.go:270`, `:274`, via `fileAtRefOrAbsent`) + call `FileAtBase`/`FileAtHead` and **no** `FileAtRef` call remains on the governed-subject + path — asserted by a source-level guard, because a green `TestForkMRNoFabricatedDelete` + against a fake that happens to serve the right bytes does not prove the call was rewritten; + (ii) the **policy** loads (`run.go:203`, `:211`, `:253`) still use + `FileAtRef(project, path, targetBranch)` and are **not** migrated — a test asserts policy is + read from the target ref of the target project even for a fork MR, so a well-meaning + "consistency" refactor onto an MR-relative accessor (which would let a fork's head reach the + policy load) fails the suite rather than silently crossing ADR-0015 §1's trust boundary. - Test: `internal/forge/port.go`, `internal/forge/conformance/`, `cmd/assent/run.go` - - Verify: `go test ./... -run TestForkMRNoFabricatedDelete` + - Verify: `go test ./... -run 'TestForkMRNoFabricatedDelete|TestPolicyLoadsFromTargetRefOnForkMR'` - Level: L1 - **REQ-E10-S02-06** — Given ADR-0021 item 7, when the port is declared, then it exposes the **authenticated identity**, and a case proves markers are recognised as our own under @@ -428,7 +448,7 @@ runs it. - **REQ-E10-S06-03** — Given `gitleaks` and D-002, when the adapter and its cassettes are committed, then no real token, org name, or private repository name appears in any fixture. - Test: `internal/forge/github/testdata/**` - - Verify: `task scrub && task check` + - Verify: `bash hack/check-sanitization.sh && task check` - Level: L0 ### E10-S07 — GitHub Snapshot `[autonomous]` @@ -633,7 +653,7 @@ runs it. `github-deferred` row in `catalog.yaml` is either flipped to `both` or **retains the deferral with a named, cited reason**; D-084 is dispositioned. -- **REQ-E10-S14-01** — Given **every one of the 15 non-deferred catalog rows is +- **REQ-E10-S14-01** — Given **every one of the 14 non-deferred catalog rows is `forge: gitlab`**, when the catalog is updated, then **every row** — not only `github-deferred` ones — carries an explicit per-adapter disposition (`both`, or a single forge **plus a cited reason**), and a test fails on any bare `forge: gitlab` row. diff --git a/openspec/specs/p5-e11-rego-backend/spec.md b/openspec/specs/p5-e11-rego-backend/spec.md index 93244270..92b12284 100644 --- a/openspec/specs/p5-e11-rego-backend/spec.md +++ b/openspec/specs/p5-e11-rego-backend/spec.md @@ -61,7 +61,8 @@ second tier real. the loader tier alongside CEL compilation; module *evaluation* is pure computation and may live in core only once S04's sandbox makes that true by construction. -**Scope**: (S01) the tier-1 ceiling, recorded with concrete exceeding rules; (S02) additive +**Scope**: (S00) the deterministic-budget feasibility spike, in a nested throwaway module so it +adds no dependency; (S01) the tier-1 ceiling, recorded with concrete exceeding rules; (S02) additive `rego:` leaf in the policy schema; (S03) module loading + compile-time errors; (S04) OPA capability sandbox (D-013); (S05) input binding to the identical `EvaluationInput`; (S06) deterministic evaluation budget; (S07) violations → findings with explicit obligation-proof @@ -89,17 +90,36 @@ loader, E3's lint hard-error framework, E6's `assent test` harness, the committe **New**: `rego:` leaf, OPA integration + capability file, deterministic budget, violation mapping. -**Executability**: S01–S13 all **`[autonomous]`** — hermetic, no infrastructure. S02, S04, +**Executability**: S00–S13 all **`[autonomous]`** — hermetic, no infrastructure. S02, S04, S06, S07 are **engine-grade** (frozen-schema change, sandbox, determinism, decision polarity) and additionally **`[maintainer LGTM]`**: S02 changes a published contract and S04/S06/S07 are the decision path itself. -**Dependency order**: S01 → S02 → S03 → S04 → S05 → S06 → S07 → S08 → {S09, S10} → S11 → S12 -→ S13. **Do first: S01** — the ceiling document determines whether the backend's shape is -right; building it without one reproduces the speculative-generality risk D-012 existed to -prevent. **S04 is blocked on an operator answer** (judgment call (d)): the rule-7 boundary -question decides which package the evaluator lives in and which gate enforces it. S01–S03 are -unblocked and can run while that answer is pending. +**Dependency order**: {S00, S01} → S02 → S03 → S04 → S05 → S06 → S07 → S08 → {S09, S10} → S11 +→ S12 → S13 — **14 stories, S00–S13.** **Do first: S00 and S01**, which are independent of each +other and of judgment call (d), so both can run immediately and in parallel. S01 (the ceiling +document) determines whether the backend's shape is right; building it without one reproduces +the speculative-generality risk D-012 existed to prevent. S00 (the budget spike) determines +whether the backend can be bounded at all. + +**What is blocked, and by what — stated precisely, because the obvious reading is wrong:** + +- **S02 is blocked on S00**, not merely sequenced after it. S02 changes a **published + contract**; if S00 returns "no deterministic budget exists", the `rego:` leaf that should be + announced is the `phase: observe`-only shape, not the gating shape (REQ-E11-S00-03). +- **S03 and S04 are both blocked on judgment call (d)** — this corrects an earlier reading of + this spec which said S01–S03 were unblocked. They are not. **S03 is the story that *effects* + the narrowing (d) governs**: REQ-E11-S03-03 adds OPA to `go.mod`, and S03's own Test paths + sit in `internal/core/policy/**` — inside the D-123 guarded tree. Landing S03 while (d) is + open converts rule 7's decision-path guarantee from link-enforced to capability-enforced + **silently and green**, because — as (d) documents — *neither* purity gate is transitive. + Deciding a hard-rule narrowing by merging a story is exactly what rule 6 forbids. (d) also + decides *which package* the evaluator lives in, so S03–S08's `internal/core/policy` paths are + written against **(d1)** and must be re-pathed if the operator answers **(d2)**. +- The operator's 2026-08-10 answer — *accept and pin OPA* — disposes of (d)'s **supply-chain** + half and rejects **(d3)**. It does **not** discriminate **(d1) from (d2)**: (d2) also accepts + and pins OPA while keeping the guarded tree OPA-free. That half remains open and is the one + that unblocks S03. ## Judgment calls (decide-and-log / operator) @@ -110,16 +130,20 @@ silently ignore a rule it cannot evaluate). This is backward-compatible and deli forward-**in**compatible, and S02 records it in `API_STABILITY.md` as an announced additive change — no `apiVersion` bump. -(b1) **⚠️ SPIKE S06 BEFORE S02 — it is a serial chokepoint on an API that may not exist.** -Adversarial review, marked verify-not-verified: OPA's public `rego` package may bound -evaluation only via `context.Context` cancellation, with **no supported deterministic -instruction/step budget**. If so, judgment call (b) below cannot be satisfied as written — and -the current order (S05→S06→…) would stall the epic at story 6 of 13 with the **schema already -changed (S02)** and the **OPA dependency already added (S03)**. Mitigation, in order of -preference: (i) spike S06's feasibility before S02 lands anything irreversible; (ii) if no -deterministic budget exists, the fallback is that Rego-backed rules are restricted to -`phase: observe` — they report but never gate — so the epic still lands something honest; (iii) -what is **not** acceptable is relaxing (b). +(b1) **DECIDED — the feasibility spike is story E11-S00 and S02 is blocked on it.** Adversarial +review, marked verify-not-verified: OPA's public `rego` package may bound evaluation only via +`context.Context` cancellation, with **no supported deterministic instruction/step budget**. If +so, judgment call (b) below cannot be satisfied as written — and an S05→S06 order would stall +the epic at story 7 of 14 with the **schema already changed (S02)** and the **OPA dependency +already added (S03)**. **Resolution:** mitigation (i) is adopted and given a story rather than +left as a preference — **E11-S00**, which runs first, carries REQ IDs and a DoD like any other +story, and is deliberately built in a **nested throwaway module** so it can answer the question +without adding OPA to `go.mod` (which judgment call (d) has not yet authorised). Fallback +(ii) — if no deterministic budget exists, Rego-backed rules are restricted to `phase: observe`, +reporting but never gating — is retained and is now REQ-E11-S00-03, which also re-scopes S02's +published schema change to match, because an observe-only leaf is a different contract from a +gating one. (iii) stands unchanged: relaxing (b) is **not** acceptable, and "timeout → BLOCK" is +not a resolution. (b) **DECIDED — evaluation is bounded by a deterministic budget, never a wall-clock timeout.** Per rule 7: the bound is an OPA evaluation-step/instruction budget that yields the identical @@ -174,7 +198,17 @@ made silently by a story. *Supply chain, separately:* OPA is a large dependency with a large transitive tree on a project shipping cosign signing, SLSA-grade provenance, `govulncheck`, and Scorecard. It materially changes binary size, vulnerability surface, and `renovate` load. S03 pins it and -records the size delta. **Both halves are recorded as D-141's open sub-question.** +records the size delta. + +*Status (2026-08-10) — the two halves have diverged and must not be conflated:* +- ✅ **Supply-chain half: ANSWERED — accept and pin.** The operator accepted + `github.com/open-policy-agent/opa` as a dependency. **(d3) is rejected** by that answer. +- 🔴 **Mechanism half: STILL OPEN — (d1) vs (d2), and this is the half that blocks S03.** + Accepting the dependency does not say *where the evaluator lives* or *which gate enforces + rule 7*: **(d2) also accepts and pins OPA** while keeping the guarded tree OPA-free. Reading + "accept and pin" as settling (d) would silently choose (d1) — the exact silent hard-rule + narrowing this judgment call exists to prevent. Per REQ-E11-S04-04 the answer needs an + **ADR-0011/rule-7 amendment plus a `D-nnn` row**, whichever way it goes. (e) **DECIDED — Rego modules are policy, and load from the target ref like all policy.** ADR-0010/ADR-0015's trust rules apply unchanged: a module is loaded from the target ref, @@ -190,6 +224,53 @@ human dependency. --- +### E11-S00 — Spike: does OPA expose a deterministic evaluation budget? `[autonomous · spike]` + +- **Goal**: answer, against a pinned OPA version and with a runnable reproduction, whether + OPA's public API can bound evaluation by a **machine-independent instruction/step count** + rather than by `context.Context` cancellation (wall clock). +- **Why first**: judgment call (b) requires exactly that budget, and judgment call (b1) records + that the API may not exist. If it does not, the epic's shape changes — and every later story + is downstream of that. Ordering this after S02/S03 would discover it with the **published + schema already changed** and the **OPA dependency already in `go.mod`**: both irreversible in + the annoying direction. This story exists so that discovery is free. +- **Dependencies**: none. It is deliberately **not** blocked on judgment call (d) — see + REQ-E11-S00-01, which is what makes that true. +- **Definition of done**: `docs/planning/spikes/spike-d-rego-budget.md` records the verdict, the + pinned OPA version it was established against, the exact API surface examined, and the + reproduction command — and the repository's own `go.mod`/`go.sum` are byte-unchanged. + +- **REQ-E11-S00-01** — Given judgment call (d) is **open** and is precisely the question of + whether OPA may enter this module's dependency graph, when the spike is written, then it + lives in its **own nested module** (`hack/spikes/rego/go.mod`) and the repository's root + `go.mod`/`go.sum` gain **no OPA entry**. Go excludes a directory carrying its own `go.mod` + from the parent module's `./...`, and this repository has **no `go.work`** (verified), so the + spike is unbuildable by `task check` and cannot smuggle the adoption (d) has not yet + authorised. Spiking a dependency is not adopting it. + - Test: `hack/spikes/rego/go.mod`, root `go.mod`, root `go.sum` + - Verify: `git diff --exit-code -- go.mod go.sum && go list ./... | grep -c 'spikes/rego' | grep -qx 0 && task check` + - Level: L0 +- **REQ-E11-S00-02** — Given the question is empirical, when the spike runs, then it either + (i) **names the public API** that bounds evaluation by a machine-independent count and + demonstrates an **identical outcome and identical budget consumption** across N≥100 runs and + across at least two `GOMAXPROCS` settings — the property S06 will later have to gate on — or + (ii) records that **no such API exists** in the pinned version, with the surface examined + enumerated so the finding is falsifiable rather than an absence-of-evidence claim. + - Test: `hack/spikes/rego/`, `docs/planning/spikes/spike-d-rego-budget.md` + - Verify: `cd hack/spikes/rego && go test ./...` + - Level: L0 +- **REQ-E11-S00-03** — Given the verdict routes the epic, when it is recorded, then it states + the consequence explicitly and a `D-nnn` row captures it **before S02 changes the published + schema**: outcome (i) → judgment call (b) stands unamended and S06 is buildable as written; + outcome (ii) → the (b1)(ii) fallback is adopted, Rego-backed rules are restricted to + `phase: observe`, and **S02's schema change is re-scoped accordingly** — a `rego:` leaf that + can only ever observe is a *different published contract* from one that can gate, and + shipping the gating shape first would announce a capability the epic cannot deliver. Under + no outcome is this resolved by "timeout → BLOCK" (judgment call (b), (iii)). + - Test: `docs/planning/spikes/spike-d-rego-budget.md`, `docs/decisions/decisions.md` + - Verify: manual review + - Level: L0 + ### E11-S01 — Record the tier-1 ceiling with concrete exceeding rules `[autonomous]` - **Goal**: a written, reviewable statement of what CEL *cannot* express, grounded in real @@ -207,7 +288,7 @@ human dependency. be expressed within the frozen predicate-scope table (`docs/planning/predicate-scope.md`) — and no employer or internal system name appears (D-002). - Test: `docs/planning/rego-tier-ceiling.md` - - Verify: `task scrub && task check` + - Verify: `bash hack/check-sanitization.sh && task check` - Level: L0 - **REQ-E11-S01-02** — Given a shape might in fact be CEL-expressible, when the document is reviewed, then any shape found expressible in CEL is **struck from E11's scope** and @@ -218,7 +299,9 @@ human dependency. ### E11-S02 — Additive `rego:` leaf in the policy schema `[autonomous · engine-grade · maintainer LGTM]` -- **Dependencies**: S01. +- **Dependencies**: **S00** (blocking — its verdict decides whether the announced leaf is the + gating shape or the `phase: observe`-only shape, per REQ-E11-S00-03; this is a published + contract and announcing the wrong shape is not walk-back-able) and S01. - **Definition of done**: `merge-policy.schema.json`'s `leaf` becomes a `oneOf` over the existing `cel` shape and a new `rego` shape; strict-decode still rejects unknown fields and a leaf carrying **both** backends; `API_STABILITY.md` records the announced additive change; @@ -249,14 +332,26 @@ human dependency. - **REQ-E11-S02-04** — Given every prior epic's DoD was `git diff schemas/` == 0, when E11's gates run, then the schema-drift guard is **scoped**, not deleted: drift is permitted only in `merge-policy.schema.json` and only for this change; drift in any `schemas/decision/**` - file still fails. - - Test: `hack/` schema-drift guard - - Verify: `task check` + file still fails. **The guard is Go, not a shell script**: + `internal/schemadrift/drift.go`'s `CheckGitFrozenOrD088PresentationOnly` compares against + `origin/main` through a two-file fence list and is invoked from three exit-gate tests, so + scoping it means adding a **third fence plus an `Allowed…Change` validator** — not editing + `hack/`. The validator must accept *only* the additive `rego:` leaf; a fence that permits + arbitrary drift in `merge-policy.schema.json` would retire the guarantee rather than scope it. + - Test: `internal/schemadrift/drift.go`, `internal/schemadrift/drift_test.go` + - Verify: `go test ./internal/schemadrift/... && task check` - Level: L1 -### E11-S03 — Module loading and compile-time errors `[autonomous]` +### E11-S03 — Module loading and compile-time errors `[autonomous · engine-grade · maintainer LGTM]` -- **Dependencies**: S02. +- **Dependencies**: S02 **and the operator's answer to judgment call (d)** — S03 cannot start + without it. This story is where the narrowing (d) governs actually *happens*: REQ-E11-S03-03 + puts OPA in `go.mod`, and this story's Test paths are inside the D-123 guarded tree. Because + neither purity gate is transitive, S03 would land **green** while converting rule 7's + guarantee from link-enforced to capability-enforced — a hard-rule change made by merging a + story, which rule 6 forbids. (d) additionally decides **which package** the evaluator lives + in: the `internal/core/policy/**` paths below are written against **(d1)** and must be + re-pathed to the injected, unguarded package if the operator answers **(d2)**. - **Definition of done**: modules resolve from the pack directory **on the target ref** (judgment call (e)); a module that fails to compile is a **load-time hard error** (E3 lint parity), never a runtime surprise; the OPA dependency is pinned. @@ -265,8 +360,15 @@ human dependency. resolves through the **same target-ref policy load path** as YAML policy, and no second loader can read a module from the PR head — asserted by a test that places a hostile module on the head ref and proves it is not evaluated. - - Test: `internal/core/policy/rego_load.go`, `rego_load_test.go` - - Verify: `go test ./internal/core/... -run TestRegoLoadsFromTargetRef` + **Placement, because the obvious location does not compile:** `internal/core/policy`'s loader + tier is **bytes-in and pure** (`LoadMergePolicy(raw []byte)`); *all* ref-addressed reading + lives in `cmd/assent` (`run.go:203`, `:211`, `:253`), and `.golangci.yml:39` denies + `internal/forge` from `**/internal/core/**` — so a core-resident loader cannot fetch a ref at + all. `internal/core/policy/rego_load.go` therefore takes an **injected reader** supplied by + `cmd/assent`, and the hostile-module-on-the-head-ref test lives in **`cmd/assent`**, where the + ref plumbing exists. If the operator answers **(d2)**, these paths move with the evaluator. + - Test: `internal/core/policy/rego_load.go`, `rego_load_test.go`, `cmd/assent/run_rego_test.go` + - Verify: `go test ./internal/core/... ./cmd/... -run TestRegoLoadsFromTargetRef` - Level: L1 - **REQ-E11-S03-02** — Given E3's hard-error framework, when a module fails to compile or references an undefined rule, then `assent lint` reports it as a **hard error** with the @@ -276,16 +378,31 @@ human dependency. - Level: L1 - **REQ-E11-S03-03** — Given judgment call (d), when OPA is added to `go.mod`, then the version is pinned, `govulncheck` and `renovate` cover it, and the binary-size delta is - recorded in the story's notes. + recorded in the story's notes. **This REQ may not be started before (d) is answered** — it is + the adoption itself, not a consequence of it. - Test: `go.mod`, `go.sum` - Verify: `task check && govulncheck ./...` - Level: L0 +- **REQ-E11-S03-04** — Given both purity gates are **non-transitive** and (d1)'s entire premise + is that the exception stays visible, when the operator answers **(d1)**, then this story also + extends the purity guard to a **transitive** check (`go list -deps` over the guarded tree) + asserting the transitive closure contains no `net`/`net/*` **except** through the single + explicitly allowlisted OPA path — and a mutation control proves the guard goes red when a + *second* dependency pulls `net` in, so the exception cannot widen unnoticed. Without this the + narrowing is accepted but not enforced, which is strictly worse than the status quo: it reads + as governed while checking nothing. If the operator answers **(d2)** this REQ is struck and + replaced by the depguard rule denying the evaluator package from `internal/core/**`. + - Test: `internal/core/purity_test.go`, `.golangci.yml`, `hack/lint/depguard_test.sh` + - Verify: `task lint && task lint-depguard-test && go test ./internal/core/... -run TestPurity` + - Level: L1 ### E11-S04 — OPA capability sandbox `[autonomous · engine-grade · maintainer LGTM]` - **Dependencies**: S03 **and the operator's answer to judgment call (d)** — S04 cannot close without it, because (d1) and (d2) place the evaluator in different packages and gate it with - different mechanisms. Everything upstream of S04 (S01–S03) is unaffected and may proceed. + different mechanisms. **S03 is blocked on the same answer** (see S03's dependencies); the only + stories genuinely unaffected by (d) and free to proceed while it is pending are **S00, S01 + and S02**. - **Definition of done**: D-013's sandbox is real — a capability set that **denies by default** and allows an explicit, enumerated builtin list; `http.send`, `net.*`, `opa.runtime`, `time.*`, `rand.*`, and any I/O builtin are unavailable; a module using one @@ -434,14 +551,14 @@ human dependency. - **REQ-E11-S09-01** — Given E3's framework, when a pack contains a broken Rego rule, then `assent lint` exits non-zero with a positioned, contributor-legible message per failure class — one test per class. - - Test: `internal/core/lint/rego_test.go` - - Verify: `go test ./internal/core/lint/` + - Test: `internal/lint/rego_test.go` + - Verify: `go test ./internal/lint/` - Level: L1 - **REQ-E11-S09-02** — Given D-048's catalogue rules, when a Rego-backed rule is catalogued, then its entry is faithful (authored `phase`, `effectivePhase`, generated `docs.url`) and fabricates no lifecycle metadata. - - Test: `internal/core/catalogue/` - - Verify: `go test ./internal/core/catalogue/` + - Test: `internal/catalogue/` + - Verify: `go test ./internal/catalogue/` - Level: L1 ### E11-S10 — `assent test` support and goldens `[autonomous]` @@ -459,7 +576,7 @@ human dependency. - Level: L1 - **REQ-E11-S10-02** — Given E6's both-polarity coverage rule, when `--coverage` runs, then a Rego rule counts as covered only when **both** polarities are exercised. - - Test: `internal/core/testharness/` + - Test: `internal/adoptertest/` - Verify: `go test ./internal/core/...` - Level: L1 @@ -475,8 +592,12 @@ human dependency. removed, then `hack/check-migration-invariants.sh` is updated so it still forbids what remains forbidden (no `rego:` leaf in an archetype/starter pack that has not been migrated) and no longer asserts a marker that must not exist — the guard is never simply deleted. - - Test: `hack/check-migration-invariants.sh` - - Verify: `task check` + **`task check` does NOT run this guard** — `hack/check-migration-invariants.sh` is invoked + only by `.github/workflows/schemas.yml`, so a green local gate proves nothing here. Invoke it + directly, and add a mutation control proving the updated guard still goes red on an + un-migrated pack carrying a `rego:` leaf. + - Test: `hack/check-migration-invariants.sh`, `.github/workflows/schemas.yml` + - Verify: `bash hack/check-migration-invariants.sh && task check` - Level: L1 - **REQ-E11-S11-02** — Given the example was excluded from CI, when the quarantine lifts, then `examples/policies/rego/bounded_change.rego` validates, compiles under S04's capabilities, From e82efd7de0d88be793d1f0cf19b0d3c093e9d4ce Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Mon, 10 Aug 2026 16:33:10 +0200 Subject: [PATCH 8/8] :wrench: chore(release): regenerate CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe43aa1f..c89a6d35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ repository still gets a decision, never by following the link; no release carrie - :memo: docs(openspec): decompose E10 and E11 into INVEST stories - :memo: docs(openspec): fix two P0 design defects found by adversarial review - :memo: docs(openspec): correct the E10 story count to 19 after adding S00 +- :memo: docs(openspec): fix 3 P1 design defects found by independent review of PR #45 ### Testing - :white_check_mark: test(release): anchor the D-120 note check on its header sentence, not the bare token