Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ repository still gets a decision, never by following the link; no release carrie
- :memo: docs(ci): withdraw the third copy of the retracted claim (D-157)
- :memo: docs(e10): record the operator LGTM that satisfies E10-S00's GOVERNANCE marker (D-161)
- :memo: docs(planning): record the 2026-08-25 operator rulings and withdraw the stale rule-7 block (D-162..D-164)
- :memo: docs(forge): name the catalog's overstated-coverage defect and log the coverage-floor headroom (D-165)

### Features
- :sparkles: feat(audit): measure REL-03/REL-07 by real mutation run, not by name
Expand All @@ -126,6 +127,7 @@ repository still gets a decision, never by following the link; no release carrie

### Refactoring
- :recycle: refactor(audit): pin REL-03 and REL-07 by test name, demote source shape
- :recycle: refactor(forge): extract the conformance suite into an importable package with an explicit observation surface (E10-S01)

### Security
- :lock: fix(cmd): discriminate absent provider declaration from forge failure (REL-03)
Expand Down
1 change: 1 addition & 0 deletions docs/decisions/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,4 @@ project/process decisions.
| D-162 | 2026-08-25 | **P5-DEM is re-decomposed spec-first before any DEM story is implemented: all 15 stories (DEM-S00–S14) carry ZERO `Verify:` annotations, so not one of them states how it would be proven.** Operator-ruled 2026-08-25. This is a spec defect, not a per-lane gap: hard rule 4 puts specs before code, and DEM-S14 is the epic exit gate — it has nothing to aggregate if each lane invents its own gate ad hoc when it is picked up. **Rejected: annotate lane-by-lane** — cheaper per lane, but it defers the question of what the epic proves to the last story, which is precisely where this project has repeatedly found gates asserting a universal property their denominator could not establish (D-160, D-157). **Rejected: leave parked** — DEM is the adoption/demo epic and DEM-S11 is also E10-S18's live adoption target, so it is on the critical path for E10's own evidence, not optional. Scope: re-decomposition is a spec change under `openspec/` only; no DEM code lands until every DEM story has a `Verify:` annotation. Revert: restore the current 15 rows from git history; the annotations are additive and can be kept regardless. |
| D-163 | 2026-08-25 | **The recurring "dependabot reddens `main` on the CHANGELOG drift gate" failure is fixed narrowly, at the bot, NOT by enabling the drift gate on `pull_request`: OQ-30's guard STAYS and D-148 STANDS.** Operator-ruled 2026-08-25 after an initial ruling to the contrary was withdrawn on corrected premises. Incident: `cd4dac1` + `34e94cf` (codeql-action 4.37.7→4.37.8) merged green and reddened `main` in BOTH `verify` (step *changelog drift gate*) and `release-exitgate` (`task check` stage 21) — run 32817829705/32817735039; healed by PR #96. **Why not enable the gate on PRs, even though D-148's stated mechanism does not survive:** OQ-30 measured the merge-direction ordering hazard only in the lane-first-parent direction, while GitHub mints the merge ref base-first — so D-148 is defensible as PRECAUTION, not as mechanism, and this row records that explicitly rather than leaving a dead premise standing. The decisive objection is different and was missed when the question was first put: **dependabot cannot run `task changelog-write` on its own branch**, and `verify` is a REQUIRED status check with `strict=true` and `enforce_admins=true` (measured via the branch-protection API). Enabling the gate would therefore convert "main red for ~1h, healed by one regen commit" into "every dependabot PR red on a required check until a human pushes a regen commit to the bot's branch" — worse for the very incident that prompted it — while additionally exposing OQ-30's three still-untested cases (fork PRs, `pull_request_target`, force-push re-minting the merge ref) on a required check, where a false positive is unfixable by the author. **Mechanism sub-choice is deliberately NOT taken here** and is parked for the operator in `agent-context/INBOX.md`: (i) auto-regenerate on dependabot branches needs a write token on a bot-authored PR branch (`pull_request_target` or a PAT) and races dependabot's own rebase force-pushes; (ii) making the push-to-main gate tolerate bot-only `build(deps)` drift is a FAIL-OPEN in a gate, the exact species this repo has spent two waves removing, and would need both polarities plus a mutation control before it could be trusted. Until one is chosen, the standing remedy is the status quo: heal `main` with a `:memo: chore(release):` regen commit, whose subject `cliff.toml:119` skips so it does not add itself. Revert: none needed — this row changes no code; it records why the guard was not deleted. |
| D-164 | 2026-08-25 | **DOC-TRUTH — `backlog.md` rows E11-S03 and E11-S04 still said "blocked on the operator's rule-7 answer (d1/d2)" nine days after D-144 answered it, and that staleness actively misled a session.** D-144 (2026-08-16) resolves D-141's sub-question (d) to **(d1)** and ends with the words "**Unblocks E11-S03 and E11-S04**"; the two backlog rows kept their 🔴 marker and their "blocked on the operator" text regardless. Consequence, observed rather than hypothesised: the 2026-08-25 session handoff and the operator's own session-opening message both carried "the rule-7 (d1/d2) question is still parked on you" forward as fact, and the question was nearly put to the operator a second time. Same species as D-157/D-160 — a claim retained after the thing that justified it was withdrawn. Fix: both rows now cite D-144 and carry their real remaining dependency. **Generalisation deliberately not made:** no gate is added here asserting "every 🔴 row has a live blocker", because that predicate would match row TEXT, and matching text where the property is structural is the defect this project keeps re-introducing (14 P1s across two gate lanes). Revert: restore the two rows from git history. |
| D-165 | 2026-08-25 | **E10-S01 extracts the forge conformance suite into importable Go, and the extraction found that the two SHA-guard cases had NEVER run against GitLab despite their catalog rows claiming `forge: gitlab`.** **That second half is a DOC-TRUTH defect in its own right, same species as [D-157], [D-160] and [D-164]:** `catalog.yaml` asserted adapter coverage that did not exist, for two L1 cases on the SHA-guard — the single most safety-relevant property in the suite — and nothing detected it for the entire life of the file, because the only thing checking the catalog compared it against test-function NAMES, which were all present and correct. A row claiming coverage is not evidence of coverage. Scope decisions taken deliberately: **(a)** `conformance.Factory` returns a `forge.Forge`, NOT the composite `forge.RunPort` that REQ-E10-S01-01 names — `RunPort` is E10-S02's deliverable and S02 is **LGTM-gated**, so declaring it in this `[autonomous]` lane would smuggle an LGTM-gated core-contract change past the `backlog.md:557` GOVERNANCE marker; S02 depends on S01 exactly so the port change lands against an executable suite, and the substitution is then a one-line change. **(b)** Merge/approve/thread write counts are taken by a PORT-LEVEL decorator (`portcount.go`), not per adapter, because they are not observable at the transport layer for GitLab — its `MergeCAS` re-reads heads and refuses client-side, so a moved head produces ZERO merge HTTP requests and an httptest harness would report 0 attempts where the port was called once. **(c)** Cases take pins from `Fixture.Pins()` instead of literals, because GitLab synthesises its merge-result digest (`gitlab.SyntheticDigest`) — a literal digest is why those cases could only ever run on the fake. Collapsing that digest is E10-S03. **Assertions were STRENGTHENED, never downgraded (REQ-E10-S01-04):** the summary-updated-in-place check was guarded behind `if ff, ok := f.(*fake.Forge); ok` and therefore unfailable on every non-fake backend; merge ATTEMPTS are now distinguished from merges PERFORMED, so "the pre-check refused before any mutation" (0 attempts) is separable from "the atomic CAS refused" (1 attempt), which `len(Merges) == 0` alone could not express; and a `NotesUpdated() > 0` check was replaced with an exact count after a gate proved no corruption of that value could flip it. Coverage of the catalogued cases went from partial-and-asymmetric to **all five on both adapters**; the package's passing assertions went from 9 test functions to 103 subtests. **The three self-checks are the deliverable as much as the extraction is** — observed-execution catalog matching, can-fail-against-sabotage on every adapter, and per-observation load-bearing corruption; the last of these found two real weaknesses in code that had already passed review, which is the recorded lesson that reviewers must EXECUTE rather than read. **Foreseeable consequence of THIS commit, recorded here so E10-S02's author does not discover it:** the extraction moved ~700 statements from `_test.go` (uncounted) into production-classified Go whose only exerciser is the three self-check gates, and global coverage now sits at **91.1% against a 91.0% floor**. Every future E10 story that adds a conformance case also adds uncovered `t.Fatalf` assertion branches and will red the D-010/D-128 floor. The fix when it happens is NOT to delete assertions — it is either to extend the load-bearing gate to the new case (which covers those branches as a side effect, and is the reason the floor is met today) or to rule on whether a test-support package belongs in the coverage denominator at all. Logged in `agent-context/INBOX.md`. Revert: the pre-extraction `_test.go` bodies are one commit back; the gates are additive and worth keeping regardless. |
219 changes: 219 additions & 0 deletions internal/forge/conformance/backends_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
package conformance

import (
"encoding/json"
"fmt"
"net/http"
"strconv"
"strings"

"github.com/PlatformRelay/assent/internal/forge"
"github.com/PlatformRelay/assent/internal/forge/fake"
gitlab "github.com/PlatformRelay/assent/internal/forge/gitlab"
)

// backends_test.go wires the two built-in backends into the shared suite. Each is
// a Factory and nothing more — the case bodies live in importable non-test Go, so
// a third adapter (GitHub, E10-S06+) needs a file this size and no copied cases.

// ---- fake backend ----

type fakeBackend struct {
f *fake.Forge
cp *countingPort
}

func (b fakeBackend) MergeAttempts() int { return b.cp.mergeAttempts }
func (b fakeBackend) MergesPerformed() int { return b.cp.mergesPerformed }
func (b fakeBackend) Approvals() int { return b.cp.approvals }
func (b fakeBackend) ThreadsCreated() int { return b.cp.threadsCreated }
func (b fakeBackend) ThreadsResolved() int { return b.cp.threadsResolved }
func (b fakeBackend) NotesCreated() int { return b.f.NoteCreateCalls }
func (b fakeBackend) NotesUpdated() int { return b.f.NoteUpdateCalls }
func (b fakeBackend) NoteBody(id string) string { return b.f.NoteBody(id) }
func (b fakeBackend) IsResolved(id string) bool { return b.f.IsResolved(id) }
func (b fakeBackend) ThreadCount() int { return b.f.ThreadCount() }
func (b fakeBackend) BotThreadCount() int { return b.f.BotThreadCount() }
func (b fakeBackend) OpenBotThreadCount() int { return b.f.OpenBotThreadCount() }

func (b fakeBackend) SeedThread(id, author string, m forge.Marker, resolved bool) error {
b.f.SeedThread(id, author, m, resolved)
return nil
}

func (b fakeBackend) SeedNote(id, author string, m forge.Marker, body string) error {
b.f.SeedNote(id, author, m, body)
return nil
}

func (b fakeBackend) MoveTargetHead(sha string) { b.f.CurrentTargetSha = sha }

func (b fakeBackend) Pins() forge.DesiredMerge {
return forge.DesiredMerge{
SourceSha: b.f.CurrentSourceSha,
TargetSha: b.f.CurrentTargetSha,
MergeResultDigest: b.f.CurrentMergeResultDigest,
}
}

func (b fakeBackend) DriftSourceHeadAfterRead(sha string) {
b.f.AfterCurrentHeads = func(fk *fake.Forge) { fk.CurrentSourceSha = sha }
}

func fakeFactory(t TB, cfg Config) Backend {
t.Helper()
f := fake.New(cfg.BotAuthor, cfg.CurrentSourceSHA, cfg.CurrentTargetSHA, cfg.CurrentMergeResultDigest)
cp := newCountingPort(f)
b := fakeBackend{f: f, cp: cp}
return Backend{Port: cp, Fixture: b, Observer: b}
}

// ---- gitlab backend (httptest) ----

type gitlabBackend struct {
h *gitlabHarness
cp *countingPort
}

func (b gitlabBackend) MergeAttempts() int { return b.cp.mergeAttempts }
func (b gitlabBackend) MergesPerformed() int { return b.cp.mergesPerformed }
func (b gitlabBackend) Approvals() int { return b.cp.approvals }
func (b gitlabBackend) ThreadsCreated() int { return b.cp.threadsCreated }
func (b gitlabBackend) ThreadsResolved() int { return b.cp.threadsResolved }
func (b gitlabBackend) NotesCreated() int { return b.h.noteCreateCalls }
func (b gitlabBackend) NotesUpdated() int { return b.h.noteUpdateCalls }

func (b gitlabBackend) NoteBody(id string) string {
n, err := strconv.Atoi(strings.TrimPrefix(id, "note/"))
if err != nil {
return ""
}
for _, note := range b.h.notes {
if note.id == n {
return note.body
}
}
return ""
}

func (b gitlabBackend) IsResolved(id string) bool {
for _, d := range b.h.discussions {
if d.id == id {
return d.resolved
}
}
return false
}

func (b gitlabBackend) ThreadCount() int { return len(b.h.discussions) }

func (b gitlabBackend) BotThreadCount() int {
n := 0
for _, d := range b.h.discussions {
if d.author == b.h.botAuthor {
n++
}
}
return n
}

func (b gitlabBackend) OpenBotThreadCount() int {
n := 0
for _, d := range b.h.discussions {
if d.author == b.h.botAuthor && !d.resolved {
n++
}
}
return n
}

func (b gitlabBackend) SeedThread(id, author string, m forge.Marker, resolved bool) error {
return b.h.seed(id, author, m, resolved)
}

func (b gitlabBackend) SeedNote(id, author string, m forge.Marker, body string) error {
n, err := strconv.Atoi(strings.TrimPrefix(id, "note/"))
if err != nil {
return fmt.Errorf("gitlab backend: note id %q is not note/<int>: %w", id, err)
}
return b.h.seedNote(n, author, m, body)
}

func (b gitlabBackend) MoveTargetHead(sha string) { b.h.targetSHA = sha }

// Pins reports the pins an evaluation would record against THIS backend. The
// digest is synthesised by the adapter (`gitlab.SyntheticDigest`) because GitLab
// exposes no merge-result digest — which is exactly why a case carrying a literal
// digest could only ever run against the fake. Collapsing the synthetic digest
// onto a real one is E10-S03.
func (b gitlabBackend) Pins() forge.DesiredMerge {
return forge.DesiredMerge{
SourceSha: b.h.sourceSHA,
TargetSha: b.h.targetSHA,
MergeResultDigest: gitlab.SyntheticDigest(b.h.sourceSHA, b.h.targetSHA),
}
}

// DriftSourceHeadAfterRead moves the source head once the pre-check's MR read has
// been served — the TOCTOU window. Reconcile reads the heads, then MergeCAS reads
// them again; arming on the FIRST read means the second sees the moved head, so
// the atomic CAS is the thing that refuses.
func (b gitlabBackend) DriftSourceHeadAfterRead(sha string) {
b.h.afterMRRead = func(h *gitlabHarness) { h.sourceSHA = sha }
}

func gitlabFactory(t TB, cfg Config) Backend {
t.Helper()
h := newGitLabHarness(cfg.Project, cfg.MR)
h.botAuthor = cfg.BotAuthor
h.sourceSHA = cfg.CurrentSourceSHA
h.targetSHA = cfg.CurrentTargetSHA
cp := newCountingPort(h.client(t))
b := gitlabBackend{h: h, cp: cp}
return Backend{Port: cp, Fixture: b, Observer: b}
}

// ---- MR / approve / merge endpoints (E10-S01) ----
//
// The pre-extraction harness served discussions and notes only, so the two
// SHA-guard cases could never run against GitLab — they were fake-only, while
// their catalog rows said `forge: gitlab`. These three routes close that gap.

func (h *gitlabHarness) serveMR(w http.ResponseWriter, _ *http.Request) {
h.mrReads++
_ = json.NewEncoder(w).Encode(map[string]any{
"iid": 1,
"project_id": 42,
"sha": h.sourceSHA,
"source_branch": "feature",
"target_branch": "main",
})
// Fire AFTER the response is written, so this read returns the PRE-move value
// and only the NEXT one sees the drift.
if h.afterMRRead != nil {
h.afterMRRead(h)
}
}

func (h *gitlabHarness) serveBranch(w http.ResponseWriter, _ *http.Request) {
_ = json.NewEncoder(w).Encode(map[string]any{
"commit": map[string]any{"id": h.targetSHA},
})
}

func (h *gitlabHarness) approve(w http.ResponseWriter, _ *http.Request) {
h.approveCalls++
w.WriteHeader(http.StatusCreated)
_ = json.NewEncoder(w).Encode(map[string]any{"id": 1})
}

func (h *gitlabHarness) merge(w http.ResponseWriter, r *http.Request) {
h.mergePUTs++
// ?sha= is GitLab's compare-and-swap on the SOURCE head: a moved source is
// 409, no merge. Modelled faithfully so the case proves the guard, not the fake.
if got := r.URL.Query().Get("sha"); got != h.sourceSHA {
http.Error(w, "sha mismatch", http.StatusConflict)
return
}
_ = json.NewEncoder(w).Encode(map[string]any{"merge_commit_sha": "deadbeef"})
}
Loading