From d527d7d86cf9cc7de8624589666b14f2c6bb94fe Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:54:04 +0200 Subject: [PATCH 1/7] :bug: fix(docs): point every docs-site URL at the case-sensitive Pages path (DOC-02) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo is named `Assent`. GitHub repo URLs are case-insensitive; GitHub Pages paths are not — so every in-tree `platformrelay.github.io/assent/...` reference 404s. Measured live: `/assent/` -> 404, `/Assent/` -> 200. Corrected in `cmd/assent/main.go` (the URL the released `assent --help` prints), the five README links, `docs/usage/cli.md` (byte-pinned against the help text, so both halves move together), `mkdocs.yml`'s `site_url` — which drives every page's canonical link and all 63 `` entries in `sitemap.xml` — plus D-044 and the E9 spec. `internal/catalogue`'s DocsBase casing goes with them. New pin in `hack/docs/truthlag_pins_test.sh` (already wired into `task check`): every `platformrelay.github.io` occurrence repo-wide, `.go` files included, must carry `mkdocs.yml`'s `site_url` prefix. It is a prefix check, not a liveness check; the comment says so and names the known dead path. D-044's URL is corrected in place rather than superseded: a dead link inside a record is a factual defect, not a decision to reverse. --- README.md | 10 +++--- cmd/assent/main.go | 5 ++- docs/decisions/decisions.md | 2 +- docs/usage/cli.md | 2 +- hack/docs/truthlag_pins_test.sh | 41 +++++++++++++++++++++++ internal/catalogue/catalogue.go | 2 +- mkdocs.yml | 2 +- openspec/specs/p5-e9-distribution/spec.md | 4 +-- 8 files changed, 56 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index b9bd127..0c6a2ac 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@

- + assent — deterministic, policy-driven auto-merge @@ -12,7 +12,7 @@ Schemas Docs OpenSSF Scorecard - Documentation + Documentation License: Apache-2.0

@@ -28,7 +28,7 @@ configuration repositories. Drop it into a repo's CI pipeline and it turns merge into decisions: **approve, comment, request changes, or block** — based on rules *you* write in **Kyverno-style declarative YAML** with CEL predicates. -**Read the docs:** **[platformrelay.github.io/assent](https://platformrelay.github.io/assent/)** +**Read the docs:** **[platformrelay.github.io/Assent](https://platformrelay.github.io/Assent/)** — vision, architecture, ADRs, install guide, and usage walkthrough. This README is the front door; the site is the map. @@ -76,7 +76,7 @@ assent version `assent 0.0.0-dev` — even when you pin a tag (`@v0.1.0`). For a **version-stamped** binary take the Homebrew tap or a release archive: goreleaser injects the version (`-X main.version`) and the archives are checksum- and signature-verifiable. Both -routes are in [docs/usage/install.md](https://platformrelay.github.io/assent/usage/install/). +routes are in [docs/usage/install.md](https://platformrelay.github.io/Assent/usage/install/). Lint and test policies locally. Both commands take the **repository root** — `assent` appends `.assent` itself, so passing `.assent/` makes it look for `.assent/.assent`: @@ -116,7 +116,7 @@ Honest tiers post-E8 (D-104). **Core** = shipped and covered by conformance test | Topic | Link | | --- | --- | -| Documentation site | [platformrelay.github.io/assent](https://platformrelay.github.io/assent/) | +| Documentation site | [platformrelay.github.io/Assent](https://platformrelay.github.io/Assent/) | | Install (go, curl, Homebrew) | [usage/install.md](docs/usage/install.md) | | API & schema stability | [API_STABILITY.md](API_STABILITY.md) | | Security policy & CI gates | [SECURITY.md](SECURITY.md) | diff --git a/cmd/assent/main.go b/cmd/assent/main.go index c571c65..7261f1e 100644 --- a/cmd/assent/main.go +++ b/cmd/assent/main.go @@ -212,7 +212,10 @@ func usageText() string { _, _ = fmt.Fprintf(&b, " %s\n %s\n usage: %s\n", sc.name, sc.synopsis, sc.usage) } b.WriteString("\nassent run -h, assent compare -h and assent render -h list their flags.\n") - b.WriteString("Full command reference: https://platformrelay.github.io/assent/usage/cli/\n") + // The Pages path is case-sensitive and the repo is named `Assent` — the + // lowercase spelling 404s. Pinned against `mkdocs.yml`'s `site_url` by + // hack/docs/truthlag_pins_test.sh (DOC-02). + b.WriteString("Full command reference: https://platformrelay.github.io/Assent/usage/cli/\n") return b.String() } diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index f6f0740..0b6a071 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -48,7 +48,7 @@ project/process decisions. | D-041 | 2026-07-27 | **P4-E1-S10 walking skeleton GREEN on live gitlab.com.** The real GitLab REST v4 forge adapter (`internal/forge/gitlab`) + `assent run` orchestration landed on `main` @ `4addc3d` (independently reviewed APPROVE; all four attacks — unintended-write, SHA-guard/synthetic-digest honesty, policy-from-target-ref, token redaction — actively driven and held). Proven end-to-end against the live lab `konrad.heimel/assent-lab`: (M1) MR!1 `partitions 6→3` → REVIEW → one resolvable thread with the ADR-0019 marker; (M2) two reruns → ZERO new threads (live rerun-idempotence, REQ-S10-02 analogue); (M3) MR!2 `partitions 6→12` with `--arm` → APPROVE → approve + SHA-pinned merge (merged `960c6dd`, pinned to evaluated source `9a816d4`); unarmed APPROVE → advisory-only, zero writes. Each run emitted a schema-valid DecisionRecord with honest `mergeResultDigest: null` + `capabilityGap` (gitlab plain-merge has no merge-result digest; the CAS uses a synthetic source+target digest that never leaks into the record). `--arm` is a SANDBOX override — it does NOT satisfy the D-034 real-protected-source verification (S05 INSECURE-PLACEHOLDER still deferred); `readPipelineDescription` is never wired into the merge path. Evidence: `docs/decisions/evidence/p4-e1-s11-adoption/`. Revert: `git revert 4addc3d`. | | D-042 | 2026-07-27 | **D-012 adoption gate SATISFIED / P4-E1-S11 done — Phase-4 exit gate CLOSED.** A real gitlab.com repository (`konrad.heimel/assent-lab`, the D-037-designated lab — a real GitLab project with real API/merge semantics, not a synthetic testcontainer fixture) ran assent on LIVE merge requests: MR!1 (REVIEW, resolvable thread) and MR!2 (APPROVE, real SHA-pinned merge `960c6dd`). DecisionRecords retained under `docs/decisions/evidence/p4-e1-s11-adoption/`; MRs: https://gitlab.com/konrad.heimel/assent-lab/-/merge_requests/1 and /2. With the S01–S12 autonomous slices (D-039) + S10 live green (D-041) + this adoption record, **the P4-E1 walking-skeleton exit gate is closed; Phase 5 (E1–E9) is unblocked.** FOLLOW-UP (S10 review F1, P2): `assent run` diffs only the single governed subject, so the `.assent/**`→`assent-policy` BLOCK routing is reachable only at the S07 engine tier, not yet on the live adapter path — the full-engine phase (E1) should enumerate the MR's changed-file set so the self-vouch BLOCK is exercised live (today the live self-vouch protection is policy-from-target-ref, which holds and was verified). Revert: factual milestone; to reopen, revert the S10 lane. | | D-043 | 2026-08-03 | **Visual identity = Decision Gate (operator-approved direction B).** The primary mark is a bold arched lowercase `a` / decision boundary with coral and cobalt paths converging through its center. Ship both a standalone mark (no writing) and a horizontal `assent` lockup, plus reproducible GitHub and Open Graph social cards and favicon/raster exports. Palette: ink `#172033`, cobalt `#316BFF`, coral `#FF5C4D`, warm white `#F8F7F2`. | -| D-044 | 2026-08-03 | **Documentation site = MkDocs Material on GitHub Pages.** Documentation changes build with `mkdocs build --strict` in pull requests; pushes to `main` deploy the same artifact to `https://platformrelay.github.io/assent/`. The README links the site and exposes only badges backed by workflows or repository metadata that exist today. Dependencies are pinned with hashes in `docs/requirements-docs.txt`. | +| D-044 | 2026-08-03 | **Documentation site = MkDocs Material on GitHub Pages.** Documentation changes build with `mkdocs build --strict` in pull requests; pushes to `main` deploy the same artifact to `https://platformrelay.github.io/Assent/`. The README links the site and exposes only badges backed by workflows or repository metadata that exist today. Dependencies are pinned with hashes in `docs/requirements-docs.txt`. | | D-045 | 2026-08-03 | **Security/OpenSSF hardening batch + GitHub Pages enabled (operator-requested, modeled on MKurator/Kollect).** (1) Enabled GitHub Pages (Actions source) via `gh api`, greening the D-044 docs deploy that had failed `404 — Pages not enabled`. (2) Added `SECURITY.md` (private vuln reporting via GitHub Security Advisories + email; posture grounded in enforced CI gates, not overclaimed) and `.github/CODEOWNERS` (`* @konih` + security-sensitive path routing). (3) Added three pinned, least-privilege workflows: `codeql.yaml` (SAST, `go`+`actions` matrix — closes P4-CODEQL), `scorecard.yaml` (OpenSSF Scorecard, weekly + publish + SARIF upload), and schedule-only `vulncheck.yaml` (weekly govulncheck sweep; per-push govulncheck already in `verify.yaml`, not duplicated). All `actionlint`-clean; independently reviewed before merge. Residual (operator): enable branch protection + required status checks on `main` (today `main` is unprotected, so these gates are advisory). Revert: delete the five files + `gh api --method DELETE /repos/PlatformRelay/assent/pages`. | | D-046 | 2026-08-03 | **P5-E2-S06 — `comment` is a decision-NEUTRAL soft-points channel for NON-required obligations only.** Implementing the ADR-0007 aggregation tail (author-declared `rule.points` accrue per firing; `sum(points) <= binding.risk.threshold` gates APPROVE as decision order #4) requires a firing that accrues points WITHOUT independently forcing REVIEW/BLOCK — otherwise the threshold can never be the deciding factor (block/challenge/require-review each already lower the decision before #4 is reached). Chose to make `comment` that channel: per ADR-0007's own effect table `comment` "Blocks merge? no". This **diverges** from the walking-skeleton `internal/core/aggregate.Aggregate` (where `effectDecision(comment) == REVIEW`); the new `Cover` path uses `firingEffectDecision` (comment→neutral, others→`effectDecision`). **Fail-safe guard (ADR-0017 §2 supersedes ADR-0007's coverage line):** neutrality applies ONLY to a NON-required obligation. A REQUIRED obligation (in `binding.require`) is satisfied-to-arm — an unproven one lowers the decision regardless of effect (`comment` still REVIEWs there), else a required obligation proved by a `comment`-onFailure rule that FIRED would fail OPEN to APPROVE on an unproven obligation. The frozen `merge-policy` schema permits `comment` on `onFailure.effect`, so this fail-open was live-reachable via authored policy, not latent. Points always accrue per firing regardless of the decision mapping. D-016 golden byte-identical (its required obligations use block/require-review, already lowering). Guard test: `TestRequiredObligationCommentFiringNeverApproves`. Revert: drop `firingEffectDecision` (restore `effectDecision` for all effects in `Cover`) — but then the S06 threshold never gates a firing-bearing MR. | | D-047 | 2026-08-04 | **P5-E3-S01 — `assent lint` is a subcommand + ingests tolerantly (fail-many).** (a) `assent lint` is a SUBCOMMAND of the single `assent` binary (reuses `main.go` dispatch + the `.assent/**` discovery), not a second `assent-lint` binary — an alias is a thin future OQ if wanted. (b) Lint does NOT reuse the strict E2 loader alone (which fail-fast-aborts on the first unknown-field/unknown-enum/missing-phase error — refusing the exact malformed packs lint exists to diagnose): it decodes each doc BEST-EFFORT into the same `policy.*` types AND runs the strict loader separately, capturing its refusal as ONE `schema-invalid` diagnostic located to the offending doc, so a malformed pack still yields ALL diagnostics in one run. This tolerant fail-many accumulator is the load-bearing architecture every later E3 check plugs into. **Known limitation (documented, not a runtime fail-open):** the S01 obligation-coverage check ignores `phase`, so an obligation proven only by a `phase:off`/`observe` rule passes the static "covered" check — at runtime `cover()` marks covered only at `PhaseEnforce`, so such an obligation is unproven → REVIEW/fail-safe; the missing-phase root cause is caught by E3-S02 `no-implicit-enforce-phase`. Revert: n/a (design record). | diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 62bc648..08e67f2 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -55,7 +55,7 @@ Commands: usage: assent help assent run -h, assent compare -h and assent render -h list their flags. -Full command reference: https://platformrelay.github.io/assent/usage/cli/ +Full command reference: https://platformrelay.github.io/Assent/usage/cli/ ``` ## assent run diff --git a/hack/docs/truthlag_pins_test.sh b/hack/docs/truthlag_pins_test.sh index 32cc68b..ead8fa4 100755 --- a/hack/docs/truthlag_pins_test.sh +++ b/hack/docs/truthlag_pins_test.sh @@ -15,6 +15,9 @@ # DOC-10 the meta-plan Phase-5 epic table covers E1..E9 and does not bind a # deferred-tier concept (Rego, GitHub adapter) to one of those numbers. # DOC-11 the `go install` caveat names the version it actually prints. +# DOC-02 every docs-site URL in the tree — INCLUDING .go files, since the released +# binary prints one — carries mkdocs.yml's `site_url` prefix. The Pages path +# is case-sensitive (the repo is `Assent`), so a lowercase spelling 404s. # plus docs/adr/README.md's status column agrees with each ADR's own Status row. # # Every check prints PASS or FAIL and the script exits 1 if any failed, so a @@ -149,6 +152,44 @@ for f in README.md docs/usage/install.md; do fi done +# --- DOC-02: every docs-site URL agrees with mkdocs.yml's site_url ---------------- +# GitHub *repo* URLs are case-insensitive; **Pages paths are not**. The repo is named +# `Assent`, so `platformrelay.github.io/assent/...` 404s — and one such URL is compiled +# into the released binary (`assent --help`), which is why this sweep covers .go files +# and not just markdown. `mkdocs build --strict` cannot catch it: it does not resolve +# external URLs, and the DOC-05 pin above matches only relative links. +# +# SCOPE LIMIT, deliberately: this compares the PREFIX against site_url. It proves the +# host and the case-sensitive repo segment are right; it does NOT prove the path +# resolves. A known instance of the gap: `internal/catalogue`'s DocsBase mints +# `rules//`, and no `rules/` space exists on the site (DOC-03) — +# correct prefix, still a 404. Liveness would need network I/O in `task check`. +SITE_URL="$(grep -m1 '^site_url:' mkdocs.yml | sed -e 's/^site_url: *//' -e 's/[[:space:]]*$//')" +if [[ -z "$SITE_URL" ]]; then + fail "DOC-02: mkdocs.yml has no site_url — the docs-URL pin has no authority to compare against" +else + # Strip the trailing slash so the prefix also matches `rules`. + SITE_PREFIX="${SITE_URL%/}" + url_checked=0 + url_bad=0 + while IFS= read -r hit; do + [[ -z "$hit" ]] && continue + url_checked=$((url_checked + 1)) + case "$hit" in + *"$SITE_PREFIX"*) ;; + *) echo " $hit" >&2; url_bad=$((url_bad + 1)) ;; + esac + done < <(git grep -n 'platformrelay\.github\.io' -- . ':(exclude)hack/docs/truthlag_pins_test.sh' 2>/dev/null) + + if [[ "$url_checked" -eq 0 ]]; then + fail "DOC-02: no platformrelay.github.io URLs found anywhere — the site_url pin would be vacuous" + elif [[ "$url_bad" -ne 0 ]]; then + fail "DOC-02: $url_bad docs-site URL(s) do not carry mkdocs.yml's site_url prefix '$SITE_PREFIX' (Pages paths are case-sensitive)" + else + pass "DOC-02: all $url_checked docs-site URLs carry the site_url prefix '$SITE_PREFIX'" + fi +fi + # --- ADR index status agrees with each ADR's own Status row ------------------------ adr_checked=0 for adr in docs/adr/0*.md; do diff --git a/internal/catalogue/catalogue.go b/internal/catalogue/catalogue.go index 7611b9f..52bf2c5 100644 --- a/internal/catalogue/catalogue.go +++ b/internal/catalogue/catalogue.go @@ -42,7 +42,7 @@ import ( // DocsBase is the docs-site root the generated docs.url is minted under (the // D-044 MkDocs site). A rule's docs.url is DocsBase + "/" + its stable ID. -const DocsBase = "https://platformrelay.github.io/assent/rules" +const DocsBase = "https://platformrelay.github.io/Assent/rules" // CapabilityApprovalEvidence is the capability a require-review rule needs: it is // satisfiable only by forge-proven ApprovalEvidence (ADR-0017 §3), never a bare diff --git a/mkdocs.yml b/mkdocs.yml index 99c6fb3..c6da578 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -1,6 +1,6 @@ site_name: assent site_description: Deterministic, policy-driven auto-merge for self-service repositories. -site_url: https://platformrelay.github.io/assent/ +site_url: https://platformrelay.github.io/Assent/ repo_url: https://github.com/PlatformRelay/assent repo_name: PlatformRelay/assent copyright: Copyright © 2026 Konrad Heimel diff --git a/openspec/specs/p5-e9-distribution/spec.md b/openspec/specs/p5-e9-distribution/spec.md index 102eed8..2a21dc4 100644 --- a/openspec/specs/p5-e9-distribution/spec.md +++ b/openspec/specs/p5-e9-distribution/spec.md @@ -16,7 +16,7 @@ SHA-pinned Actions, Dependabot, `SECURITY.md`, `CODEOWNERS`, `cliff.toml` (minim - **Security CI mostly done:** `.github/workflows/{verify,codeql,scorecard,vulncheck}.yaml` are pinned and green (D-045); S04 is an **audit + residual gap** lane, not a second CodeQL install. - **Docs pipeline live:** `mkdocs.yml` + `docs/requirements-docs.txt` + `task docs-build` + - `docs.yaml` deploy to `https://platformrelay.github.io/assent/` (D-044) — but nav still exposes + `docs.yaml` deploy to `https://platformrelay.github.io/Assent/` (D-044) — but nav still exposes `docs/planning/*` (oss-playbook anti-pattern #3); S08 fences product-only pages. - **Changelog seed exists:** root `cliff.toml` with gitmoji parsers; no `CHANGELOG.md` sync CI yet. - **Sibling patterns:** `kollect-render/.goreleaser.yaml` (CLI ldflags + cross-compile) and @@ -459,7 +459,7 @@ that** the Phase-5 distribution gate closes. checksum verify in CI; docs site builds product nav; backlog marks E9 spec authoritative. **Infra-gated half:** tag `v0.1.0` (D-108); S05–S06 publish signed assets; S07b tap (if ready); install via curl script + `go install` + Homebrew; cosign/SBOM verified live; -`https://platformrelay.github.io/assent/` live with install page. Record **D-111** (E9 exit gate +`https://platformrelay.github.io/Assent/` live with install page. Record **D-111** (E9 exit gate closed) when both halves done. **Dependencies**: E9-S01..S12. From 0e28e75fac3c388adfc254d75b7fca926e48f1c1 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:54:21 +0200 Subject: [PATCH 2/7] :bug: fix(render): stop minting a 404 documentation link into every MR thread (DOC-03) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_render.go` attached `catalogue.DocsBase + "/" + pack + "/" + rule` to every rule on the run path, rendered as a `📖 [Full documentation](…)` link in the contributor's thread. No `rules/` space exists on the docs site — measured, `/rules` and `/rules/` both 404 against a 63-page sitemap — so the one affordance a blocked contributor clicks was dead on every finding. Casing does not fix it; the URL space does not exist in either case. The run path now carries only the rule's authored `docs.url`, and `renderDocsSection` already omits the line when that is empty: no link beats a broken one. Today the fallback is dormant BY SCHEMA, not merely unused — the frozen v1alpha1 merge-policy schema is `additionalProperties: false` over eight keys, so an authored rule-level `docs:` is rejected at load (audit ARCH-08). Both polarities pinned, since "no link" is also what deleting the feature would produce: absent -> no link and no docs-site URL anywhere in the body; authored -> that URL verbatim. Mutation-proven — restoring the minting reds both subtests. Building the `/rules/` pipeline stays out of scope. `assent catalogue`'s JSON still carries DocsURL (D-017 B10 field set, an openspec-level change); the constant now documents the dead space instead of implying it resolves. --- cmd/assent/run_render.go | 22 +++++-- cmd/assent/run_render_test.go | 106 ++++++++++++++++++++++++++++++++ internal/catalogue/catalogue.go | 11 ++++ 3 files changed, 135 insertions(+), 4 deletions(-) diff --git a/cmd/assent/run_render.go b/cmd/assent/run_render.go index fc2d801..f86023e 100644 --- a/cmd/assent/run_render.go +++ b/cmd/assent/run_render.go @@ -1,7 +1,8 @@ package main import ( - "github.com/PlatformRelay/assent/internal/catalogue" + "strings" + "github.com/PlatformRelay/assent/internal/change" "github.com/PlatformRelay/assent/internal/core/aggregate" "github.com/PlatformRelay/assent/internal/core/decision" @@ -50,14 +51,27 @@ func rulesMetaFromPolicy(mp *policy.MergePolicy) map[string]render.RuleMeta { if mp == nil { return nil } - pack := mp.Metadata.Name out := make(map[string]render.RuleMeta, len(mp.Spec.Rules)) for _, r := range mp.Spec.Rules { - stableID := pack + "/" + r.Name + // DOC-03: do NOT mint `catalogue.DocsBase + "/" + pack + "/" + r.Name` + // here. That URL space does not exist on the docs site — measured, both + // `/rules` and `/rules/` 404 — so every finding posted into a + // contributor's MR thread carried a dead "Full documentation" link, on the + // one affordance a blocked contributor actually clicks. The run path now + // carries only the rule's AUTHORED docs.url; `renderDocsSection` omits the + // line entirely when it is empty, so no link beats a broken one. + // + // Today that fallback is DORMANT BY SCHEMA, not merely unused: the frozen + // v1alpha1 merge-policy schema is `additionalProperties: false` over + // [effect, match, message, name, onFailure, phase, points, prove], so an + // authored rule-level `docs:` is REJECTED at load and `policy.Rule.Docs` + // can never be populated from a conformant pack (audit ARCH-08). Adopters + // cannot supply a URL here yet — the honest present-day behaviour is + // therefore "no documentation link on the run path", which is the point. out[r.Name] = render.RuleMeta{ Message: r.Message, Docs: render.RuleDocs{ - URL: catalogue.DocsBase + "/" + stableID, + URL: strings.TrimSpace(r.Docs.URL), }, } } diff --git a/cmd/assent/run_render_test.go b/cmd/assent/run_render_test.go index 3551e82..a058e6a 100644 --- a/cmd/assent/run_render_test.go +++ b/cmd/assent/run_render_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "github.com/PlatformRelay/assent/internal/catalogue" "github.com/PlatformRelay/assent/internal/change" "github.com/PlatformRelay/assent/internal/core/aggregate" "github.com/PlatformRelay/assent/internal/core/decision" @@ -167,3 +168,108 @@ func TestBuildDesiredSummaryUsesRenderer(t *testing.T) { }) } } + +// DOC-03 (audit 2026-08-09): the run path must not MINT a documentation URL. +// +// `internal/catalogue.DocsBase + "/" + pack + "/" + rule` was attached to every +// finding on the run path and rendered as `📖 [Full documentation](…)` in the +// contributor's MR thread — but no `rules/` space exists on the docs site +// (measured: `/rules` and `/rules/` both 404), so the one affordance +// a blocked contributor clicks was dead on every finding of every MR. +// +// Both polarities, because "no link" is also what deleting the feature outright +// would produce and that must not pass as a fix: +// +// absent → no link line at all (and no docs-site URL anywhere in the body); +// authored → the AUTHORED url, verbatim, is the one that renders. +// +// The authored case is unreachable from a conformant pack TODAY: the frozen +// v1alpha1 merge-policy schema is `additionalProperties: false` over +// [effect, match, message, name, onFailure, phase, points, prove], so a rule-level +// `docs:` is rejected at load (audit ARCH-08). It is pinned here anyway so that +// closing ARCH-08 wires an authored URL through instead of re-minting a dead one. +func TestRunPathDocsLinkIsAuthoredNeverMinted(t *testing.T) { + for _, tc := range []struct { + name string + docsURL string + wantLink bool + wantInURL string + }{ + {name: "absent_authored_url_renders_no_link", docsURL: "", wantLink: false}, + {name: "authored_url_renders_verbatim", docsURL: "https://docs.example.test/rules/partitions", wantLink: true, wantInURL: "https://docs.example.test/rules/partitions"}, + } { + t.Run(tc.name, func(t *testing.T) { + result := aggregate.Result{ + Decision: aggregate.DecisionReview, + Findings: []aggregate.Finding{{ + Rule: "partitions-must-not-shrink", + Obligation: "non-destructive", + Effect: aggregate.EffectChallenge, + Subject: "topic-registry:orders.events.v1", + Code: "partition-count-shrunk", + }}, + } + pins := decision.Pins{ + ToolVersion: "test", + ToolDigest: "sha256:abc", + PolicySha: "sha256:def", + SourceSha: "src", + TargetSha: "tgt", + MergeResult: decision.SkeletonMergeGap(), + } + report, err := decision.Build(result, pins) + if err != nil { + t.Fatalf("decision.Build: %v", err) + } + recordJSON, err := report.MarshalRecord() + if err != nil { + t.Fatalf("MarshalRecord: %v", err) + } + + mp := &policy.MergePolicy{ + Metadata: policy.Metadata{Name: "topic-safety"}, + Spec: policy.MergePolicySpec{ + Rules: []policy.Rule{{ + Name: "partitions-must-not-shrink", + Message: "Partitions {{ old }} -> {{ new }}", + Docs: policy.RuleDocs{URL: tc.docsURL}, + }}, + }, + } + bind := &policy.Binding{Environment: "prod"} + cs := change.ChangeSet{Changes: []change.Change{{ + Path: "/partitions", + Kind: change.KindModify, + Old: "5", + New: "3", + }}} + rctx := buildRenderContext(render.DefaultOptions(), mp, bind, cs, nil, forge.MRInfo{}, "") + + cfg := runConfig{project: "42", mr: "7", subject: "file:topics/orders.yaml"} + info := forge.MRInfo{SourceSHA: "src", TargetSHA: "tgt"} + desired, _ := buildDesired(cfg, info, cfg.subject, []byte("head-bytes"), result, recordJSON, false, report.Presentation, rctx) + if desired.Thread == nil { + t.Fatal("expected thread for REVIEW") + } + body := desired.Thread.Body + + // The dead space must never reach a contributor. The needle is derived + // from catalogue.DocsBase rather than written out, so it tracks that + // constant instead of going stale beside it — and so this file holds no + // literal docs-site URL for the DOC-02 site_url pin to trip over. + for _, dead := range []string{catalogue.DocsBase, "github.io", "/rules/topic-safety"} { + if strings.Contains(body, dead) { + t.Errorf("run path minted a docs URL containing %q into the MR thread:\n%s", dead, body) + } + } + + hasLink := strings.Contains(body, "Full documentation") + if hasLink != tc.wantLink { + t.Errorf("Full documentation link present = %v, want %v:\n%s", hasLink, tc.wantLink, body) + } + if tc.wantInURL != "" && !strings.Contains(body, tc.wantInURL) { + t.Errorf("authored docs.url %q did not reach the thread body:\n%s", tc.wantInURL, body) + } + }) + } +} diff --git a/internal/catalogue/catalogue.go b/internal/catalogue/catalogue.go index 52bf2c5..1ee8ccb 100644 --- a/internal/catalogue/catalogue.go +++ b/internal/catalogue/catalogue.go @@ -42,6 +42,17 @@ import ( // DocsBase is the docs-site root the generated docs.url is minted under (the // D-044 MkDocs site). A rule's docs.url is DocsBase + "/" + its stable ID. +// +// KNOWN DEAD SPACE (audit DOC-03, recorded not fixed): the site publishes 63 +// pages and NONE of them is under `rules/` — measured, `/rules` and +// `/rules/` both 404. So every URL minted here resolves to nothing. The +// contributor-facing surface no longer carries it: `cmd/assent/run_render.go` +// stopped attaching this to MR-thread findings. What remains is the local +// `assent catalogue` JSON report, whose DocsURL is part of the D-017 B10 field +// set — narrowing that is a contract change and needs an openspec proposal, so +// it is deferred rather than done here. The casing IS load-bearing and correct: +// the repo is `Assent` and Pages paths are case-sensitive, so the lowercase +// spelling would 404 on the host as well as the path. const DocsBase = "https://platformrelay.github.io/Assent/rules" // CapabilityApprovalEvidence is the capability a require-review rule needs: it is From 455f14c90d42349f32dd77d6204c7207402d33bb Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:54:45 +0200 Subject: [PATCH 3/7] :memo: docs(cli): record that --checkout is not bound to the evaluated commit (SEC-01) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This release hardens `--checkout` extensively (D-133 refuses any symlink anywhere) and the page already names "run without `--checkout`" as the first remedy — so it reads as though the flag has been made sound. It has not. Measured, not transcribed: `cmd/assent/checkout.go` contains zero occurrences of `SHA`/`Sha`, `run.go` has no step comparing the checkout to `pins.sourceSha`, and the pins and the compare-and-swap merge come from `info.SourceSHA` — the forge's view. So assent judges the tree it is handed and the forge merges the commit it knows about, with nothing relating the two. Stated as a property of how the checkout is CONSTRUCTED, which is what it is: an operator who builds `head/` from the merge-request head SHA and cancels superseded pipelines is unaffected. Deliberately claims no mitigation on assent's part — it probes no such project setting, and citing one it never reads is the pattern the audit found three times. --- docs/usage/cli.md | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 08e67f2..754bcb2 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -194,7 +194,9 @@ What to do about it: - **Run without `--checkout`.** The forge snapshot then enumerates the changed-file set and none of the above applies — symlinks in the repository become irrelevant. This is the supported way to evaluate a repository that legitimately contains one; the trade-off is the - snapshot-completeness behaviour described next. + snapshot-completeness behaviour described below. Note that the symlink refusal hardens what + the checkout may *contain*; it does not establish which *commit* the checkout is — see + *Known limitation: the checkout is not bound to the evaluated commit* below. - **Or provision a symlink-free checkout** for `base/` and `head/`. The two side directories `base/` and `head/` may themselves be symlinks — they are @@ -205,6 +207,29 @@ the same reason. Loosening the symlink refusal will mean folding it into the opa fail-safe **REVIEW** path, so such a repository gets a decision someone must look at — never by following the link. There is no release commitment for that today. +### Known limitation: the checkout is not bound to the evaluated commit + +**assent judges the tree you hand it, and nothing verifies that tree is the commit the forge +will merge.** With `--checkout` the local tree is the sole authority for the bytes under +judgment and for the changed-file set (D-077); the SHAs that pin the approval and the +compare-and-swap merge come from the forge's view of the merge request. The two are never +compared — `assent run` has no step that hashes the checkout or matches it against +`pins.sourceSha`. If they disagree, assent decides on one tree and the forge merges another. + +This is a property of **how the checkout is constructed**, not a fault that fires on every +run. Two operator obligations make it a non-issue, and assent performs neither of them for +you: + +- **Construct `head/` from the merge-request head commit** — the SHA the pipeline was + triggered for — rather than from a branch tip resolved at clone time. +- **Cancel superseded pipelines on a new push.** A push landing between the clone and the + decision leaves assent judging the older tree while the forge merges the newer head. + This is a project setting on your forge; assent never reads it and never reports on it, + so do not treat a green `assent doctor` as evidence that it is set. + +Runs without `--checkout` are not exposed to this: the forge snapshot is then both the +enumerator and the thing the pins describe. + ### Checkout-less runs and enumeration completeness Without `-checkout`, the forge snapshot's changed-file list is the only thing that can From 199fa89eb8f6092474f651f664269e7c1a06a8c4 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:54:45 +0200 Subject: [PATCH 4/7] :memo: docs(adr): correct four ADR claims the code does not support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0013 Amendment 1 told authors that ordering raw text "graduates to the Rego escape hatch (ADR-0002)". There is no Rego backend: zero `opa`/`rego` module dependencies, no `rego` property in any frozen v1alpha1 policy schema, no backend selector. The pointer is replaced by the coercion idioms the same section already gives (`int(...)`, `double(...)`, `timestamp(...)`), the design-taste sentence is kept, and Rego is named as planned (E11, D-012) with no implementation today. `internal/core/aggregate/evaluate.go` carried the same false pointer in a comment — a second copy the finding did not name — and is corrected with it. This matters now because D-131's compatibility note ships in the v0.2.0 Release body addressed to anyone whose policy orders text on purpose: the tag manufactures the readership that clicks through. ADR-0002's Status read a plain "Accepted (v2)" while README calls the Rego backend "Locked — E11" and the C4 page "PLANNED — E11" — the ADR was the only surface claiming it exists. Annotated. ADR-0006's Status asserted "CI e2e default: GitLab CE testcontainer". No CI job boots one: `test/e2e/**` is behind `//go:build e2e` and `verify.yaml`'s only e2e touch is `go vet -tags e2e ./...`, which compiles and runs nothing. ADR-0019 §"Multi-replica HA" carries the ADR set's only unmet MUST: `assent doctor` MUST emit `duplicate_prevention:`. The value is computed (`doctor_forge.go:19`) and dropped by `emitDoctorReport`. Annotated as unmet rather than emitted: this is one instance of audit ARCH-11 (doctor prints essentially none of its typed report), so emitting one field would half-close it and leave the report inconsistent — and a user-visible CLI output change does not belong in a docs-truth lane. The ADR index rows for 0002/0006/0009/0019 are updated too. D-135 claimed its Status extension made the ADR-0009 gap visible on the index; it did not, because the index is a separate file with its own column — a false claim inside a correction about a false claim. Leaving 0002/0006/0019's rows plain would have reproduced exactly that. --- .../0002-policy-frontends-rego-declarative.md | 2 +- docs/adr/0006-testing-strategy.md | 2 +- docs/adr/0013-assert-syntax-and-backend.md | 14 +++++++++++--- ...lication-marker-reconciliation-protocol.md | 19 ++++++++++++++++++- docs/adr/README.md | 8 ++++---- internal/core/aggregate/evaluate.go | 5 +++-- 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/docs/adr/0002-policy-frontends-rego-declarative.md b/docs/adr/0002-policy-frontends-rego-declarative.md index 30feaee..6e2fadd 100644 --- a/docs/adr/0002-policy-frontends-rego-declarative.md +++ b/docs/adr/0002-policy-frontends-rego-declarative.md @@ -2,7 +2,7 @@ | | | | --- | --- | -| **Status** | Accepted (v2 — supersedes the "two parallel frontends" draft of this ADR; P2-E5) | +| **Status** | Accepted (v2 — supersedes the "two parallel frontends" draft of this ADR; P2-E5). **The pluggable-backend half is UNBUILT as of 2026-08-09**: the YAML envelope and the CEL/`assert` backend are Core, but the **Rego backend is deferred to E11** and nothing selects a backend today — no `opa`/`rego` module dependency, no backend field in any frozen v1alpha1 policy schema. `README.md`'s maturity table (*Rego backend — Locked, E11*) and [`docs/architecture/c4-container.md`](../architecture/c4-container.md) (*PLANNED — E11*) are the accurate surfaces; this ADR is the design, not a statement of what ships. Do **not** cite Rego as an available escape hatch — see [ADR-0013](0013-assert-syntax-and-backend.md) Amendment 1 and D-012. | | **Date** | 2026-07-21 (revised) | | **Deciders** | Konrad Heimel | | **Context links** | [ADR-0003 change model](0003-canonical-change-model.md) · [ADR-0007 effects](0007-rule-effects-decision-aggregation.md) · [ADR-0008 routing](0008-change-classification-routing-scope.md) · D-006 | diff --git a/docs/adr/0006-testing-strategy.md b/docs/adr/0006-testing-strategy.md index 54fca7c..7e62e80 100644 --- a/docs/adr/0006-testing-strategy.md +++ b/docs/adr/0006-testing-strategy.md @@ -2,7 +2,7 @@ | | | | --- | --- | -| **Status** | Accepted (CI e2e default: GitLab CE testcontainer — Spike B; P2-E5) | +| **Status** | Accepted (Spike B chose the GitLab CE testcontainer as the *intended* CI e2e default; P2-E5). **NOT IN EFFECT as of 2026-08-09 — no CI job boots one.** `test/e2e/**` is behind `//go:build e2e`, and `verify.yaml`'s only e2e touch is `go vet -tags e2e ./...`, which compiles the wiring and runs nothing; the L3 tier is armed by `ASSENT_E2E_GITLAB` against an operator-provided instance ([`test/e2e/README.md`](https://github.com/PlatformRelay/assent/blob/main/test/e2e/README.md)). Live-forge coverage is an **open assurance gap** (D-035/D-038), not a control this project currently runs. | | **Date** | 2026-07-21 | | **Deciders** | Konrad Heimel | | **Context links** | [ADR-0005 forge](0005-forge-abstraction-gitlab-first.md) · [openspec/config.yaml](https://github.com/PlatformRelay/assent/blob/main/openspec/config.yaml) | diff --git a/docs/adr/0013-assert-syntax-and-backend.md b/docs/adr/0013-assert-syntax-and-backend.md index 8e2bdd2..c2b6790 100644 --- a/docs/adr/0013-assert-syntax-and-backend.md +++ b/docs/adr/0013-assert-syntax-and-backend.md @@ -124,9 +124,17 @@ Consequences for authors: - Ordering **quoted** numerics means coercing first: `int(new) >= int(old)` (already the idiom in this repo's tests), or `double(...)`. Comparing ISO-8601 dates means `timestamp(a) < timestamp(b)`. -- **Ordering raw text is no longer expressible in tier-1** and graduates to the Rego escape - hatch (ADR-0002) — consistent with this ADR's design taste: don't grow a programming language - in YAML. There is no exempt spelling: `string(a) < string(b)` and the byte-wise +- **Ordering raw text is no longer expressible in tier-1**, and the supported answer is the + coercion above — `int(...)`, `double(...)`, `timestamp(...)` — which is also the honest one: + text that is *meant* to be ordered is almost always a number or a date wearing quotes, and + naming the type is what makes the comparison mean something. This is consistent with this + ADR's design taste: don't grow a programming language in YAML. **There is no escape hatch + today.** ADR-0002's pluggable Rego backend is **planned, not built** — E11 in the deferred + tier (D-012); there is no `opa`/`rego` module dependency, no `rego` property anywhere in the + frozen v1alpha1 policy schemas, and no backend selector a policy could set. A predicate that + genuinely needs ordering over opaque text has no in-product answer at present; express the + intent structurally (`match`, equality, membership) or model the field as the type it is. + There is no exempt spelling: `string(a) < string(b)` and the byte-wise `bytes(a) < bytes(b)` are the same lexical sort as `a < b` and are refused with it. No policy in the corpus, the comparison suite, or either dogfood pack ordered text. - The refusal set is exactly CEL's two text-shaped types, `string` and `bytes`. Every other diff --git a/docs/adr/0019-publication-marker-reconciliation-protocol.md b/docs/adr/0019-publication-marker-reconciliation-protocol.md index 7b1d15a..a141a14 100644 --- a/docs/adr/0019-publication-marker-reconciliation-protocol.md +++ b/docs/adr/0019-publication-marker-reconciliation-protocol.md @@ -2,7 +2,7 @@ | | | | --- | --- | -| **Status** | Accepted (D-030 — Phase-3 freeze review) | +| **Status** | Accepted (D-030 — Phase-3 freeze review). **One MUST is unmet:** `assent doctor` never emits `duplicate_prevention:` — see the *Implementation status* note below and [D-138](../decisions/decisions.md). | | **Date** | 2026-07-24 | | **Deciders** | Konrad Heimel | | **Context links** | [ADR-0011](0011-core-ports-and-contracts.md) (`UpsertComment`/`SyncThreads`) · [ADR-0012](0012-presentation-templates-debug.md) (finding-key / marker comments) · [ADR-0015](0015-trust-boundaries-merge-integrity.md) §6 (serve dedup) · [ADR-0016](0016-presentation-theming.md) §1 (renderer-owned marker region) · [ADR-0017](0017-contract-model-obligations.md) §7 (`Reconcile`) · D-007 · D-017 (B6) · frozen contract [marker grammar](../contracts/p3-e5-publication-protocol/marker-grammar.md) | @@ -90,6 +90,23 @@ claim `single-writer-serialized` when it cannot verify the serialization mechani default on ambiguity is `unserialized-best-effort`. The setup walkthrough's CI step and the doctor checklist must state this requirement explicitly (P3-E5-S04). +> **Implementation status (2026-08-09, [D-138](../decisions/decisions.md)) — the +> `duplicate_prevention:` MUST above is UNMET.** The value is computed and typed all the way to +> the report — `internal/forge/precondition.go` defines both constants and defaults to +> `unserialized-best-effort`, and `cmd/assent/doctor_forge.go:19` copies it into +> `PreconditionReport.DuplicatePrevention` — and then `emitDoctorReport` (same file, `:85-99`) +> prints only the arm-eligible verdict and the refusal reasons. **No `assent doctor` output +> contains the string `duplicate_prevention`.** The *safe-default* half of the MUST does hold: +> nothing can claim `single-writer-serialized` without the serialization mechanism, because +> `PreconditionFromCapabilities` seeds the field to `unserialized-best-effort`. What is missing +> is the emission, so an operator cannot read the guarantee level off the tool. +> +> Recorded rather than fixed, deliberately: this is one instance of the broader +> **audit ARCH-11** — doctor computes a typed capability report and prints essentially none of +> it, against the MUSTs of two ADRs. 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; both belong +> in the v0.2.1 ARCH-11 slice with its own tests, not in a docs-truth lane. + ## Consequences - Phase-3 freeze review **accepted** this ADR (D-030); Status is **Accepted** and matches the diff --git a/docs/adr/README.md b/docs/adr/README.md index 8c80239..0d73ac9 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -13,14 +13,14 @@ supersessions by ADR-0016/0017 are noted on each ADR's status line (not full | ADR | Title | Status | | --- | --- | --- | | [0001](0001-language-go-single-binary.md) | Implementation language: Go, single static binary | Accepted | -| [0002](0002-policy-frontends-rego-declarative.md) | Policy surface: one Kyverno-style YAML envelope, pluggable expression backends | Accepted (v2) | +| [0002](0002-policy-frontends-rego-declarative.md) | Policy surface: one Kyverno-style YAML envelope, pluggable expression backends | Accepted (v2) — **pluggable half unbuilt: Rego is E11** | | [0003](0003-canonical-change-model.md) | Canonical change model for JSON / YAML / HCL-tfvars (incl. deletions & renames) | Accepted (partial: ADR-0017 §5) | | [0004](0004-plugin-architecture.md) | Plugin architecture for permission & fact providers | Accepted | | [0005](0005-forge-abstraction-gitlab-first.md) | Forge abstraction: GitLab first, GitHub second | Accepted (partial: ADR-0017 §1/§7) | -| [0006](0006-testing-strategy.md) | Testing strategy: spec-driven pyramid with real-forge e2e | Accepted | +| [0006](0006-testing-strategy.md) | Testing strategy: spec-driven pyramid with real-forge e2e | Accepted — **no CI e2e job; L3 is operator-armed** | | [0007](0007-rule-effects-decision-aggregation.md) | Rule effects and decision aggregation (incl. risk points) | Accepted (partial: ADR-0017 §2/§3) | | [0008](0008-change-classification-routing-scope.md) | Change classification, ruleset routing, and rule scope | Accepted | -| [0009](0009-execution-modes.md) | Execution modes: CI, local/dry-run, explain, webhook, scan/stats | Accepted (partial: ADR-0017 §4) | +| [0009](0009-execution-modes.md) | Execution modes: CI, local/dry-run, explain, webhook, scan/stats | Accepted (partial: ADR-0017 §4) — **substantially unimplemented: only `run` exists** (D-135) | | [0010](0010-config-files-repo-layout.md) | Configuration files and governed-repo layout | Accepted (partial: ADR-0017 §2/§5) | | [0011](0011-core-ports-and-contracts.md) | Core Go ports and public contracts (draft shapes) | Accepted (partial: ADR-0017 §1/§7) | | [0012](0012-presentation-templates-debug.md) | Presentation: comment rendering, expandable details, docs links, rule debug | Accepted (override → ADR-0016) | @@ -30,5 +30,5 @@ supersessions by ADR-0016/0017 are noted on each ADR's status line (not full | [0016](0016-presentation-theming.md) | Presentation theming: config knobs, slots, CEL messages, render contract | Accepted | | [0017](0017-contract-model-obligations.md) | Contract model: governed subjects, required obligations, typed facts, preconditioned reconciliation | Accepted | | [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) | +| [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) | diff --git a/internal/core/aggregate/evaluate.go b/internal/core/aggregate/evaluate.go index 34bd278..9ae6786 100644 --- a/internal/core/aggregate/evaluate.go +++ b/internal/core/aggregate/evaluate.go @@ -121,8 +121,9 @@ func evalLeaf(env *cel.Env, in EvaluationInput, ch EvalChange, envLabel, expr st // // Deliberate ordering stays expressible by coercing first — `int(new) >= int(old)` // (already the repo's idiom), `double(...)`, or `timestamp(a) < timestamp(b)` for -// ISO-8601 dates. Ordering raw text is NOT expressible in tier-1 `assert` and -// graduates to Rego (ADR-0013 Amendment 1). +// ISO-8601 dates. Ordering raw text is NOT expressible in tier-1 `assert`, and +// coercion is the whole answer — ADR-0002's Rego backend is PLANNED (E11, D-012), +// not built, so there is no escape hatch to graduate to (ADR-0013 Amendment 1). // // Purity/determinism: watching only reads values that were computed anyway; it // adds no clock, randomness or I/O, and the recorded hit is the FIRST in From fefc26f1a79587f637a4a1acd263de7930ad77a6 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:55:00 +0200 Subject: [PATCH 5/7] :memo: docs: retract the unsafe advice at D-134's stopping point; dry-run is not shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D-134's "Corrected surfaces" clause recommended staying advisory by leaving a precondition unmet "or `phase: observe`" — advice the same row retracts ~2,000 words later as ARM-08, "false in the unsafe direction, twice over". A reader who stops at the natural stopping point left with the retracted advice. The clause now carries the retraction inline. Factual-record correction, not a decision reversal. D-041 gets a `Superseded by D-134` pointer appended; its content is byte-unchanged per GUIDELINES (supersede, don't edit). `vision.md` listed a "local dry-run" among the SHIPPED modes. Measured in this worktree: `assent run --dry-run` prints `flag provided but not defined: -dry-run` and exits 2. Corrected to one shipped mode and four planned. Note that the commit which introduced this (f2f59ed) was itself a docs-truth fix — it split the paragraph into shipped vs planned and promoted dry-run into the shipped half, making the false claim stronger and load-bearing. --- docs/decisions/decisions.md | 4 ++-- docs/vision.md | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index 0b6a071..323e7f0 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -45,7 +45,7 @@ project/process decisions. | D-038 | 2026-07-26 | **Local kind GitLab lab is authorized but deferred.** Operator: do **not** land the long-lived `hack/kind` setup/status/teardown + `task kind-up` lab now; **do** record that it **should** be done and **is allowed** (Spike B / OQ-6 already picked testcontainer for CI and kind for local/demo). An early implementation was reverted the same day. When a lane picks this up: promote `hack/spikes/e2e/boot-kind.sh` into an idempotent lab under `hack/kind/` (reuse Omnibus slim config + `kind load image-archive`), wire Task targets, keep CI on testcontainer. Until then the scaffold (`kind-config.yaml` + README) and spike harness remain. Revert: cancel the authorization or implement immediately via a dedicated lane. | | D-039 | 2026-07-26 | **P4-E1 autonomous walking-skeleton slices complete on `main` (through `aaeff79`).** Landed via TDD → gate → fresh independent review → ff-merge: S03 obligations aggregation; S04 DecisionRecord+PresentationModel report; S06+S08 Reconcile (post one resolvable thread / approve + SHA-pinned merge against the in-memory fake); S07 trust-boundary goldens (`.assent/**`→assent-policy BLOCK, SHA-guard rejection, tokenless evaluation path); S09 e2e harness wiring (compiles/vets under `-tags e2e`, skips without infra); S12 determinism double-run + rerun/crash-then-rerun idempotence + deterministic lowest-numeric-forge-ID duplicate-repair with additive `PublicationReceipt.repairs`. Two engine lanes needed 2 review rounds each: S03 (a real multi-entry non-numeric fail-open reaching APPROVE on an unproven obligation — closed by omitting unbound `old`/`new` from the CEL activation so any reference errors to REVIEW) and the forge lane (an untested atomic MergeCAS SHA-guard branch masked by the pre-check — closed by a TOCTOU racing-fake test). **D-034 UNCHANGED / not tripped:** S08 gates writes on injected `ArmEligible` test data against the fake, never the INSECURE-PLACEHOLDER env reader; real protected-source verification before a real approve/merge write remains the open E4/E5 hard gate. **S12 F1 deferred to E4/E12:** an all-unparseable-forge-ID canonical tie is scan-order-dependent but unreachable (real note IDs are numeric); optional one-line secondary-string-tiebreak hardening. Remaining Phase-4 exit-gate = S10+S11 (operator-run, D-035). Revert: `git revert` the slice commits `76d70c7 ae60727 0f0c726 4429581 57179f7 aaeff79`. | | D-040 | 2026-07-26 | **Provider contract-test `-race` exec deadline 1s→5s — no data race exists.** Two independent investigations found NO data race in `hack/spikes/provider`; the intermittent `signal: killed` in `TestContract` was its 1s `CallExec`/`CallHTTP` deadline exceeded under `-race` instrumentation (the parent stdin-copy goroutine is CPU-starved, so the `toyexec` child blocks past 1s). Fixed by matching the already-stable `isolation_test.go` 5s deadline — a correct under-provisioned-harness-deadline fix, NOT race-masking (there is no race to mask). Commit `a7b3579`. Revert: `git revert a7b3579`. | -| D-041 | 2026-07-27 | **P4-E1-S10 walking skeleton GREEN on live gitlab.com.** The real GitLab REST v4 forge adapter (`internal/forge/gitlab`) + `assent run` orchestration landed on `main` @ `4addc3d` (independently reviewed APPROVE; all four attacks — unintended-write, SHA-guard/synthetic-digest honesty, policy-from-target-ref, token redaction — actively driven and held). Proven end-to-end against the live lab `konrad.heimel/assent-lab`: (M1) MR!1 `partitions 6→3` → REVIEW → one resolvable thread with the ADR-0019 marker; (M2) two reruns → ZERO new threads (live rerun-idempotence, REQ-S10-02 analogue); (M3) MR!2 `partitions 6→12` with `--arm` → APPROVE → approve + SHA-pinned merge (merged `960c6dd`, pinned to evaluated source `9a816d4`); unarmed APPROVE → advisory-only, zero writes. Each run emitted a schema-valid DecisionRecord with honest `mergeResultDigest: null` + `capabilityGap` (gitlab plain-merge has no merge-result digest; the CAS uses a synthetic source+target digest that never leaks into the record). `--arm` is a SANDBOX override — it does NOT satisfy the D-034 real-protected-source verification (S05 INSECURE-PLACEHOLDER still deferred); `readPipelineDescription` is never wired into the merge path. Evidence: `docs/decisions/evidence/p4-e1-s11-adoption/`. Revert: `git revert 4addc3d`. | +| D-041 | 2026-07-27 | **P4-E1-S10 walking skeleton GREEN on live gitlab.com.** The real GitLab REST v4 forge adapter (`internal/forge/gitlab`) + `assent run` orchestration landed on `main` @ `4addc3d` (independently reviewed APPROVE; all four attacks — unintended-write, SHA-guard/synthetic-digest honesty, policy-from-target-ref, token redaction — actively driven and held). Proven end-to-end against the live lab `konrad.heimel/assent-lab`: (M1) MR!1 `partitions 6→3` → REVIEW → one resolvable thread with the ADR-0019 marker; (M2) two reruns → ZERO new threads (live rerun-idempotence, REQ-S10-02 analogue); (M3) MR!2 `partitions 6→12` with `--arm` → APPROVE → approve + SHA-pinned merge (merged `960c6dd`, pinned to evaluated source `9a816d4`); unarmed APPROVE → advisory-only, zero writes. Each run emitted a schema-valid DecisionRecord with honest `mergeResultDigest: null` + `capabilityGap` (gitlab plain-merge has no merge-result digest; the CAS uses a synthetic source+target digest that never leaks into the record). `--arm` is a SANDBOX override — it does NOT satisfy the D-034 real-protected-source verification (S05 INSECURE-PLACEHOLDER still deferred); `readPipelineDescription` is never wired into the merge path. Evidence: `docs/decisions/evidence/p4-e1-s11-adoption/`. Revert: `git revert 4addc3d`. **Superseded by D-134 as of 2026-08-09** — specifically the operator-facing claim "unarmed APPROVE → advisory-only, zero writes": `--arm` gates nothing since `c05cde0`, so an unarmed run on a forge-probe-eligible MR approves and merges. Everything else in this row stands, and per GUIDELINES (supersede, don't edit) the row above is byte-unchanged; this pointer is appended so a reader arriving here is not left with the stale guarantee. | | D-042 | 2026-07-27 | **D-012 adoption gate SATISFIED / P4-E1-S11 done — Phase-4 exit gate CLOSED.** A real gitlab.com repository (`konrad.heimel/assent-lab`, the D-037-designated lab — a real GitLab project with real API/merge semantics, not a synthetic testcontainer fixture) ran assent on LIVE merge requests: MR!1 (REVIEW, resolvable thread) and MR!2 (APPROVE, real SHA-pinned merge `960c6dd`). DecisionRecords retained under `docs/decisions/evidence/p4-e1-s11-adoption/`; MRs: https://gitlab.com/konrad.heimel/assent-lab/-/merge_requests/1 and /2. With the S01–S12 autonomous slices (D-039) + S10 live green (D-041) + this adoption record, **the P4-E1 walking-skeleton exit gate is closed; Phase 5 (E1–E9) is unblocked.** FOLLOW-UP (S10 review F1, P2): `assent run` diffs only the single governed subject, so the `.assent/**`→`assent-policy` BLOCK routing is reachable only at the S07 engine tier, not yet on the live adapter path — the full-engine phase (E1) should enumerate the MR's changed-file set so the self-vouch BLOCK is exercised live (today the live self-vouch protection is policy-from-target-ref, which holds and was verified). Revert: factual milestone; to reopen, revert the S10 lane. | | D-043 | 2026-08-03 | **Visual identity = Decision Gate (operator-approved direction B).** The primary mark is a bold arched lowercase `a` / decision boundary with coral and cobalt paths converging through its center. Ship both a standalone mark (no writing) and a horizontal `assent` lockup, plus reproducible GitHub and Open Graph social cards and favicon/raster exports. Palette: ink `#172033`, cobalt `#316BFF`, coral `#FF5C4D`, warm white `#F8F7F2`. | | D-044 | 2026-08-03 | **Documentation site = MkDocs Material on GitHub Pages.** Documentation changes build with `mkdocs build --strict` in pull requests; pushes to `main` deploy the same artifact to `https://platformrelay.github.io/Assent/`. The README links the site and exposes only badges backed by workflows or repository metadata that exist today. Dependencies are pinned with hashes in `docs/requirements-docs.txt`. | @@ -134,7 +134,7 @@ project/process decisions. | D-130 | 2026-08-08 | **The resource→owner registry loads from the TARGET ref first; the checkout is a fallback ON ABSENCE ALONE, never a shadow (GUIDELINES §Safety 2+3 / ADR-0015 §1).** Found while assessing the D-129 sibling: `loadResourceOwnerRegistry` tried `repoFS` BEFORE `client.FileAtRef(targetRef)`, and under `--checkout` `repoFS` is the merge request's own head tree — so an MR could ship `governance/owners.yaml` naming its author as owner of the resource it is changing and satisfy an `ownership` obligation with a registry it wrote itself. That registry is a decision input (it decides WHO MAY APPROVE), so it must come from the target ref like every other deciding document; only the material under judgment comes from the MR branch. Order is now target ref → (on **absence only**) checkout → error. Reproduced by mutation: with the old order the loaded owner is `attacker`, with the new order `team-payments`, same inputs (`TestResourceOwnerRegistryLoadsFromTargetRef`). Compat: repos/hermetic runs whose target ref carries no registry still load the checkout copy (`TestResourceOwnerRegistryFallsBackToCheckout`); both sides missing stays an error, never an empty map that would make every resource unowned. No test, example or dogfood fixture depended on the old precedence (the live resource-owner path had no cmd-level test at all before this row). `loadResourceOwnerRegistry` now takes a one-method `refFilePort` instead of the whole `forgePort`, which is what made the precedence unit-testable. **Amendment (independent security review, same lane) — the fallback is gated on ABSENCE, not on "the forge answered badly", and this row's own "never a shadow" claim was false as first landed.** The code fell back to the checkout on **every** `FileAtRef` error, so a forge that merely failed — 401, 429, 500, 502, 503, an expired token, a proxy hiccup — handed the who-may-approve document to the merge request's own head tree, with **no error surfaced** and nothing in the decision to show it happened. Reproduced: a 503 on the **modify-plus-transient-error** shape (a plain value diff on an existing registry) resolves `owner = "attacker"`. The 404 shape is separately mitigated — a whole-file registry *add* folds opaque → REVIEW — which is precisely why the modify shape was the live one. That is a fail-OPEN under GUIDELINES §Safety 2 and the exact shadow this row claimed was impossible. The gate is now `errors.Is(err, forge.ErrNotFound)`, the **neutral port sentinel** (`internal/forge/port.go`, already wrapped with `%w` by the GitLab adapter) — deliberately **not** `gitlab.ErrNotFound`, which would red the ARCH-02 / depguard boundary gate (D-123). **Outcome, corrected — this amendment first said "any non-absence error → … → fail-safe REVIEW", and that is NOT what ships.** The error is not swallowed into an unresolved fact: it propagates `loadResourceOwnerRegistry` → `providerCallFor` → `resolveRunFacts` → `orchestrate`, so **the run ABORTS**. Measured on the live `runRun` path with a forge that 503s the registry read only — one run against a fresh MR: exit code **1**, nothing on stdout, and on stderr `assent run: resolve providers: provider "owner": resource-owner registry "governance/owners.yaml" at ref "main": gitlab: get file "governance/owners.yaml" at ref "main": unexpected status 503`; `approvals=0 merges=0 discussionsPosted=0 notesPosted=0 notesUpdated=0`. That is more conservative in what assent WRITES — no APPROVE, no arming, nothing written — but **NOT strictly more conservative in effect**, and "strictly" was itself an overclaim minted by the previous correction of this sentence. It matters because this row exists to inform a future safety choice, so a false premise here is worse than the error it replaced. **Measured** — two `runRun` calls against ONE `fakeGitLab`, which models the forge and the MR rather than the run, so the carry-over is genuine MR state and not test leakage: run 1 serves the registry, decides `APPROVE`, and posts one summary note the MR displays as `**✅ Policy evaluation** … **Decision:** APPROVE`; run 2 then 503s on the registry read and leaves the MR EXACTLY as run 1 left it — `notes=1` still carrying that `✅ … APPROVE` body **unrevised**, `notesUpdated=0`, `discussionsPosted=0`, no thread — on top of the red job above. The same two runs with ONLY the registry content changed, so the resource is unknown and run 2 decides `REVIEW`, instead **upsert that same note** to `**📋 Policy evaluation** … **Decision:** REVIEW` (`notesUpdated=1`, note count still 1) **and post one unresolved discussion** (`discussionsPosted=1`). The ABORT shape was measured in both arming postures — arming eligible (run 1 also writes `approvals=1 merges=1`) and arming refused/advisory (`approvals=0 merges=0`, so the stale `✅` summary is the ONLY thing the MR carries) — with the identical stale-summary result; the REVIEW comparison above was run in the arming-eligible posture only. **Read, not measured** (a fake forge cannot demonstrate merge behaviour): that unresolved discussion is what HOLDS the merge wherever assent can arm at all, because `PreconditionFromCapabilities` refuses arming unless `only_allow_merge_if_all_discussions_are_resolved` is enabled (`internal/forge/precondition.go:63-68` → `ArmEligible` → `reconcileApproveMerge`'s `ErrArmingRefused`, `internal/forge/forge.go:780`; polarity pinned by `TestRunApproveUnarmedNoWrite`) — and the abort supplies none. So the gap is not the UX gap this row previously described ("there is nothing in the MR explaining why"): the artifact a human reads still says APPROVE, and nothing assent wrote holds the merge. It is **not** an auto-merge risk — an aborted run arms nothing and writes nothing — it is a **stale-green risk on the human path**, and it bites precisely on the rerun-after-a-green-run shape. The rest of the earlier wording stands: a contributor hitting a flaky forge gets a red CI job with **no DecisionRecord and no thread at all**, not a reviewable REVIEW thread. Anyone changing this must choose between the two deliberately; today it is the abort. Test: `TestResourceOwnerRegistryTransientForgeErrorNeverFallsBackToCheckout` (500/502/503/429/401), mutation-proven red at `owner = "attacker"` before the gate landed, plus `TestResourceOwnerRegistryForgeErrorAbortsResolveRunFacts`, which pins the propagation one level up — `providerCallFor` → `loadResourceOwnerRegistry` → `resolveRunFacts`, wiring that was correct but that NO test in the tree entered (`builtin/resource-owner` appeared in no config, example, exit gate or run test), mutation-proven red on both links (`return nil, err` → `return nil, nil` in `providerCallFor`'s resource-owner arm; the `provider %q` wrap → `continue` in `resolveRunFacts`), and red for no other test in the repo. The three pre-existing registry stubs moved from `errors.New("404 file not found")` to the adapter's real wrapped shape: "any error opens the fallback" **was** the defect, so a stub asserting it would have stayed green over the fix. **Sibling checked and deliberately NOT changed, recorded so the next reviewer need not re-derive it:** the host-declaration load at the head of `resolveRunFacts`'s provider loop `continue`s on ANY `FileAtRef` error — the same absence/error conflation this amendment closed for the registry — but it is fail-safe by a DIFFERENT mechanism, re-confirmed empirically through `runRun` on an ownership policy proving `facts.owner.owner.state == 'resolved'` (declaration present + registry served from the target ref → `APPROVE` on a genuinely **resolved** owner fact, `approvals=1 merges=1`; declaration 404 → `REVIEW` with `effect: require-review`, `code: predicate.error`, one thread posted, `approvals=0 merges=0`), because a skipped provider leaves the fact ABSENT, the CEL bound reference errors, and the engine routes a predicate error to require-review — a decision and a thread, not an abort. **The earlier reading of that control was DEGENERATE, and it is corrected rather than restated.** It ran against `cmd/assent/provider_host_registry_test.go`'s host-declaration fixture, which declared output `team` at `maxAge 1h` while `builtin/resource-owner` emits `owner` at 24h (`OutputOwner` / `OwnerDeclaration()`), so `ResolveFactsChecked`'s `DeclarationsEqual` check synthesized `state:"invalid"` / "provider echoed declaration does not match host config" on every run — the APPROVE arm therefore said nothing about a healthy resolve. Measured on that fixture: the run emits `APPROVE` with ZERO findings — the same output a run with no provider configured at all would produce — because the probe rule referenced `facts.owner.team` on an MR that already carried a human approval. The mechanism (the FALSE predicate's `require-review` onFailure being satisfied by that standing approval evidence) is INFERRED, not separately measured here; its opposite polarity is pinned by `TestRunNilEvidenceRequiresReview`. Re-measured with the rule pointed at the fact the builtin actually emits: the broken fixture yields `REVIEW`/`predicate.error` with a thread and `facts used: owner.team=invalid` — indistinguishable from the 404 arm — while the corrected fixture yields `APPROVE` on a resolved fact. The fixture is corrected in this lane and the healthy resolve is pinned by `TestResourceOwnerDeclarationResolvesOwnerFact` (mutation-proven red on both the output name and the `maxAge`), so the control now discriminates. The skip, fail-safe though it is, remains **SILENT** (`pins.factsResolvedAt` is simply `{}` and no reason is emitted), so an operator cannot distinguish "the provider is not declared for this repo" from "the forge failed to serve the declaration"; noted, not fixed here. Spec: `REQ-E5-S08-03`. Revert: swap the order back, or widen the gate to any error — either reinstates the shadowing fail-open. | | D-131 | 2026-08-08 | **Engine P1 fail-open — an ordering operator over a TEXT operand is an evaluation error, not a lexical answer (ADR-0013 Amendment 1).** CEL defines `<` `<=` `>` `>=` over strings as a lexical compare, so a `when: new >= old` leaf answered a boolean whenever its operands bound as text — and both answers are wrong. Reproduced end-to-end through the production entry point (`change.Diff` → `evaldecode.DecodeCanonical` → `aggregate.Cover`) on a routine adopter shape, a QUOTED numeric: `partitions: "12"` → `"6"` is a `!!str` the differ tag-discriminates and the decoder keeps a Go string BY DESIGN, so the D-016 `partitions-must-not-shrink` rule evaluated the lexical `"6" >= "12"` = **true**, proved `non-destructive`, fired nothing, and returned **APPROVE with zero findings** — a BLOCK→APPROVE flip. The mirror case is equally wrong: a legitimate grow `"6"` → `"12"` evaluated false and BLOCKed. Second, narrower instance: a numeric literal representable as neither int64 nor float64 fell back to its STRING form in `toCEL`, so `9e399 > 1e400` was lexically true (numerically false). **Fix, at the `evalLeaf`/`toCEL` seam only:** (1) `evalLeaf` plants a watcher (a cel-go `CustomDecorator`) on every operand of every relational operator; if an operand ACTUALLY evaluates to text — a CEL `string` OR `bytes` value — the leaf returns an error → `predicate.error` → REVIEW. Value-based, not syntax-based, and deliberately so: whether `new >= old` is sound depends on the adopter's DATA, not the policy text, so **lint cannot catch this class** (`checkLeafScope`/`checkPredicateScope`/`checkFactsShape` were checked; none could) — the information exists only at evaluation. Watching (not reading post-eval `EvalState`) is what makes EVERY comprehension iteration visible, since state keeps one value per node id. Short-circuited compares never run their watcher, so a policy cannot flip to REVIEW for a compare that did not happen. The guard is applied to **both** evaluation seams: `evalLeaf` (the E2 production path) and `evalRule` (the walking-skeleton `Aggregate` path, exported and test-only today, whose env declares `old`/`new` as `StringType` and binds the RAW canonical strings — so every bare relational there was lexical by construction, with only convention mandating `int()`). One unguarded evaluator is how this class returns; same drift argument as D-055(c). Its cost-budget test doubles as the interaction proof that cel-go still charges a decorated operand (`costBombWhen` is the left operand of `> 0`; cel-go v0.30.0). (2) `toCEL` binds an unrepresentable numeric literal as a CEL error value instead of its string form; an over-int64 but float64-representable literal still binds lossily — ADR-0013 residual #1's lossy half is UNCHANGED and still live. **Behaviour change, stated plainly: a policy that previously evaluated (wrongly) now ERRORS.** Any leaf that ordered text — in either direction, at any decision — becomes `predicate.error` → REVIEW. Nothing in `examples/`, the comparison corpus, either dogfood pack or the test corpus ordered text (all relational leaves are numeric: partitions, memory_mb, replicas, `size()`), so **no golden, fixture or gate output changed**; the only changed test is `internal/evaldecode`'s mutation proof `TestStringOldNewFailsOpen`, renamed `TestUndecodedStringOldNewFailsSafe` — it asserted APPROVE to document the fail-open the decoder closes, and now asserts the discriminating pair (decoded → BLOCK + `partition-count-shrunk`; un-decoded → REVIEW + `predicate.error`), so neither layer can rot silently. **Consequence for authors:** ordering quoted numerics means coercing first — `int(new) >= int(old)` (already the repo idiom) or `double(...)`; dates use `timestamp(...)`. **Ordering raw text is no longer expressible in tier-1 `assert`, in ANY spelling** — `string(a) < string(b)` and the byte-wise `bytes(a) < bytes(b)` are the same lexical sort and are refused with the bare form — and graduates to Rego (ADR-0002 escape hatch), consistent with ADR-0013's "don't grow a programming language in YAML". Equality, `in` and the string functions are untouched. Scope call: the guard is on `evalLeaf` only, NOT on `EvalScalar` — `{{ }}` message interpolation renders text and decides nothing, so guarding it would widen the blast radius for no safety gain. Records: **no `openspec/` change entry** — `openspec/changes/` holds only its README (this repo has never used change proposals in practice), and a fail-safe correction of an existing frozen semantic is proportionate to a D-row + ADR amendment + failing-test-first. Revert: drop `textOrderGuard` and restore `toCEL`'s `x.String()` fallback — reopens the APPROVE flip. **Review close-out (three non-blocking notes, decided by the author):** (F-1) the first cut watched only `types.String`, so `bytes(a) < bytes(b)` — the IDENTICAL byte-wise compare — slipped through and `bytes(new) >= bytes(old)` over the quoted `"12"` → `"6"` shrink still answered `true`. Not accident-reachable (the defect class is a quoted numeric in adopter YAML, not a deliberate `bytes()` call), so the choice was real: exempt it as a sanctioned explicit escape hatch, or refuse it. **Refused** — the lane had already rejected exempting `string(...)` because it "re-admits the identical defect", and that reasoning applies verbatim; a documented exemption would also advertise a one-token workaround for the guard's own error message, converting "not accident-reachable" into "the documented way round the guard". The refusal set is now `string` + `bytes`, and it is provably COMPLETE: the `_<_` overloads reachable from `newEvalEnv` were enumerated empirically — `int`/`uint`/`double`/`bool`/`duration`/`timestamp` are genuine orderings and pass through, `list`/`map`/`type`/mixed-numeric have no overload at all — so ADR-0013 Amendment 1's universal claim is now true as written rather than merely intended. `TestOnlyTextShapedOperandsAreRefused` pins all three buckets and reds if the env gains a comparable type (e.g. `CrossTypeNumericComparisons`). (F-2) the guard's refusal sentence was commented "adopter-facing"; it is not — both consumers use the error only for its non-nil-ness (`coverSubject` renders the AUTHORED leaf message, `evalRule`'s caller renders none), so no surface prints it today. Comment corrected to say what the consumers do; the sentence stays plain-worded so it is fit to surface if an engine error channel is ever added. (F-3) `{{ old }}`/`{{ new }}` over an unrepresentable literal interpolate the CEL error VALUE, rendering toCEL's sentence where a number belongs. Left as-is deliberately and GOLDENED: it is authored prose, not a Go-internals leak (GUIDELINES §Contracts), it decides nothing (the leaf already errored → REVIEW), it says WHY the rule could not decide where a bare `{{ old }}` placeholder would not, and re-opening this row's explicit "guard `evalLeaf`, not `EvalScalar`" scope call for a cosmetic P3 buys no safety. The golden also pins the DIVERGENCE between the two interpolators (`expandMessage` renders the sentence; `internal/render`'s `EvalScalar` path rejects the error value), so a future unification cannot change either silently. | | D-133 | 2026-08-09 | **The local checkout is CONTENT UNDER JUDGMENT, not trusted operator input: reads are contained, symlinks are refused, and a truncated tree is never returned with a nil error.** Two production defects in `cmd/assent/checkout.go`, both reproduced end-to-end through `runRun --checkout`, both fail-OPEN. **(1) P0 truncation:** `collectTree` tolerated `fs.ErrNotExist` for the WHOLE `filepath.WalkDir`, so any mid-walk ENOENT aborted the walk and returned a silently truncated map with `err == nil`. One dangling symlink named `.aaa` (sorting before `.assent`) on an MR that adds `.assent/newpack.yaml` turned `"decision":"BLOCK"` into `"decision":"APPROVE"` with three forge operations written — the **D-042** self-vouch guard defeated by a single mode-120000 blob. **Mechanism, measured not reasoned:** `filepath.WalkDir` itself does not error on a dangling symlink (`files=34 err=` over a real cloned tree) — its `DirEntry` comes from the Lstat-based `ReadDir`. The ENOENT was raised INSIDE the callback by `os.ReadFile`, at arbitrary depth, and is indistinguishable BY KIND from "the root does not exist". The two are therefore separated by WHERE they arise, never by `errors.Is`: `os.Lstat` the root once up front (Lstat not Stat, so a dangling side symlink is a broken checkout rather than an empty side), then propagate every in-walk failure unconditionally. A dangling symlink is only the cheapest trigger — a permission error, a device node or a plain race truncate identically, so the guard is pinned by a NON-symlink test (`TestCollectFSPropagatesMidWalkNotExist`, a fixture FS that lists a file and then refuses to open it). **Reachability, also measured:** a repo seeded with `ln -s /etc/hosts .aaa` and `ln -s /nonexistent/nope .aab` commits both as mode-120000 blobs, and both `git clone` and `git worktree add` recreate them as real POSIX symlinks, the dangling one included. Applies to both sides: `ChangedFiles` unions base and head keys, so a truncated BASE hides a sibling whole-file DELETE — the E1-S08-03 fail-safe. **(2) P1 substitution:** `readIfPresent` was a bare `os.ReadFile` and `run.go:279` overrides the GOVERNED SUBJECT's base/head bytes from it, so a head-side file symlink at a legitimate in-repo path made an off-tree host file the document under judgment: `topics/orders.yaml -> /host-owned.yaml` saying `partitions: 20` APPROVEd a repo whose own head said 12 unchanged, and a host value of `31337` was rendered verbatim into the posted thread (`- matched change: /partitions modify 65536 -> 31337`) — exfiltration on the forge-facing surface. Fixed with the **D-129** idiom, one containment idiom for the codebase: `os.OpenRoot` + `(*os.Root).FS()` PLUS an explicit refusal of any candidate reached through a symlink. **Both layers are required, verified not assumed:** a root FS follows a RELATIVE symlink that resolves back inside the root (`topics/orders.yaml -> ../decoy.yaml` reads the decoy), and `os.Root`'s Lstat reports the link rather than erroring — so the refusal must be explicit; conversely the root is what refuses a leaf swapped between Lstat and open. **Refusal is an ERROR, never "absent"** — silent-absent would erase the path from the changed-file set and collapse into the EFE-S03 presence signal where nil means a whole-file lifecycle event. The presence contract is preserved exactly: `nil` = ABSENT, non-nil zero-length = PRESENT-but-empty. **Rule stated plainly: no symlink anywhere under `base/` or `head/`.** Containment is anchored AT those two directories — they are operator-provisioned and may themselves be symlinks; nothing beneath them may be. **Consequences, stated plainly:** (a) `assent run --checkout` on a real repository that legitimately contains ANY symlink now hard-errors (exit 1, zero forge writes) instead of judging it — fail-closed, but a usability regression for such repos; revisit by folding the refusal opaque (fail-safe REVIEW with a resolvable thread) rather than by weakening it; (b) non-regular files (FIFOs, sockets, devices) are refused too — previously `os.ReadFile` on a FIFO would have blocked the run forever; (c) the false `#nosec G304 G122` rationale ("operator-supplied … not remote/attacker input"; "symlink TOCTOU is out of scope … fixing it belongs to the checkout-provisioning story") is DELETED, not rewritten — it was wrong about the trust boundary and it attributed an out-of-scope claim to ADR-0008 §4, which says no such thing. ADR-0008 gains Amendment 2 stating the boundary. **Legitimate-polarity regression caught and fixed inside the lane, worth naming because `task check` was blind to it:** `fs.ValidPath` rejects a rooted name, and `run.go` computes `governed := strings.TrimPrefix(cfg.subject, "file:")` without trimming a leading slash — so `--subject file:/topics/orders.yaml` began hard-erroring. Measured on the pre-fix reader: exit 0, APPROVE (`filepath.Join` cleaned the slash away). `readIfPresent` now normalises it exactly as `anchorFromSubject` does, so the two subject readers agree; it is a normalisation, not a weakening — the name stays relative to the side root, and a `..` surviving `path.Clean` is still refused. Every fixture in the suite used the unrooted form, which is why the gate matrix said nothing; both polarities are now pinned and mutation-proven. **Known residual:** a leaf swapped for an IN-ROOT symlink between `Lstat` and open is followed; the bytes are still in-tree contributor content under the same root, so this is a within-tree substitution, not an escape — an escaping swap is refused at the syscall level (`openat …: path escapes from parent`). **Test note:** mutation testing found three guards no test could red (the walk tolerance, the root-vs-`os.DirFS` choice, and `nil = absent`, whose every existing EFE-S03 fixture omits the WHOLE side directory and so never reaches the per-file read); all three were made falsifiable and seven mutations red. **Correction, logged before merge: that sentence, and commit `78df98c`'s title "make every containment guard falsifiable", both overstated.** Two guards inside `collectFS` — the symlink refusal and the non-regular-file refusal — were still a CONJUNCTION GATE: each stayed green when removed ALONE and only removing BOTH reddened the suite; and `openCheckoutSide`'s deliberate `os.Lstat` (not `os.Stat`) had no test at all. Both are closed now, with the distinction stated rather than blurred. **The two `collectFS` arms cannot be separated BEHAVIOURALLY, and the pin says so:** `fs.WalkDir`'s DirEntry comes from the Lstat-based ReadDir, on `(*os.Root).FS()` and `fstest.MapFS` alike, so a symlink is always ALSO non-regular and the non-regular arm is a strict behavioural superset — delete the symlink arm and the tree is still refused, still exit 1, still zero forge writes. Only the WORDING differs. So the symlink arm is pinned as a MESSAGE CONTRACT (`reached through a symlink`, now a documented promise in `docs/usage/cli.md` and contributor-facing per GUIDELINES — "refusing non-regular file" does not tell an adopter their repository contains a symlink), and the non-regular arm is pinned BEHAVIOURALLY with an `fs.ModeNamedPipe` fixture that carries no symlink bit. Mutation-proven each ALONE: dropping the symlink arm yields `refusing non-regular file "LICENSE" in checkout tree head` (arm 1 reds, arm 2 green); dropping the non-regular arm collects the pipe and returns `2 entries [run/queue.pipe topics/orders.yaml]` with a nil error (arm 2 reds, arm 1 green). The `os.Lstat` pin IS behavioural: `os.Stat` follows the link, so a DANGLING side symlink stats as `fs.ErrNotExist` — indistinguishable from the one absence this package tolerates — and the broken checkout reads as an EMPTY head, enumerating every base file as a whole-file DELETE; the mutation reds with exactly `files=[.assent/p.yaml] err=`, a silent mass delete offered for judgment. Its legitimate-polarity twin (a side symlinked to a REAL directory still reads) pins Amendment 2's operator-provisioned exemption, which nothing else did. **Carrier note for consequence (a):** a limitation counts as documented only where the person hits the wall, so it is now stated at `docs/usage/cli.md`'s `--checkout` flag — scope (ANY symlink ANYWHERE under `base/` or `head/`, including one that predates the MR and that the MR never touches; measured on the BASE side with an untouched `LICENSE -> LICENSES/Apache-2.0.txt`), exit 1, zero forge writes, the verbatim refusal text, the drop-the-`--checkout`-flag escape, and the `base/`/`head/` exemption — and as a `cliff.toml` "Compatibility notes" entry, which is the only carrier reaching the published GitHub Release body (`CHANGELOG.md` is regenerated in full from commit history, so a hand-edit there is wiped by the next `task changelog-write`). Revert: restore the two `os.ReadFile` call sites and the whole-walk `fs.ErrNotExist` tolerance — reopens both fail-opens. | -| D-134 | 2026-08-09 | **`--arm` is ADVISORY-ONLY and has gated nothing since `c05cde0`; this SUPERSEDES D-041's operator-facing claim "unarmed APPROVE → advisory-only, zero writes", which was true when recorded and has been false since.** D-041 is left byte-unchanged per GUIDELINES (supersede, don't edit): its evidence was gathered on 2026-07-27 against `4addc3d`, which `git merge-base --is-ancestor 4addc3d c05cde0` confirms is an ancestor of the commit that invalidated it — so the row was accurate on its own date and this one records where it stopped being accurate, not that it was wrong on arrival. **What changed:** `c05cde0` (2026-08-04, `:sparkles: feat(run): wire forge Snapshot/Resolve on assent run (E4-S06)`) replaced `Preconditions{ArmEligible: cfg.arm}` with `ArmEligible: armEligible`, the forge-probed `forge.PreconditionFromCapabilities` value, closing D-034/D-074/D-078 per `REQ-E4-S06-05`. **That engine behaviour is CORRECT and stays** — it is pinned by a genuinely discriminating test, `TestRunForgeProbedArmingGatesWrites`, which passes no `--arm` and asserts the writes happen. What went unrecorded is the operator-facing consequence: a flag documented as required-for-writes silently became cosmetic. `git show c05cde0 -- cmd/assent/run.go \| grep BoolVar` returns nothing — the commit rewrote `buildDesired`'s own arming comment and touched neither the flag's help string nor `runRun`'s doc comment. **Measured through the production entry point `runRun` against `fakeGitLab`, not transcribed:** on a forge-probe-ELIGIBLE APPROVE fixture, `args=[]` → `exit=0 approvals=1 merges=1`, summary `decision=APPROVE arm=false → 3 forge operation(s) written`; `args=[--arm=false]` → byte-identical; `args=[--arm=true]` → `exit=0 approvals=1 merges=1`. **The flag's default is `false`, so every run that never passes `--arm` was in the state the help string described as safe.** On a probe-INELIGIBLE fixture (C3 gate absent) both `args=[--arm]` and `args=[]` give `exit=0 approvals=0 merges=0`, summary `… → advisory-only (arming precondition unmet, no approve/merge)`. **The real gate** is the default-deny `internal/forge/precondition.go`: `ArmEligible` requires ALL of C17 `ProtectedPipelineExternal` (refusal `insecure-topology`), C3/ADR-0009 `DiscussionsResolvedGate` (`discussions-gate-missing`), and C6/C7 `HasApprovalRulesAPI && Tier != TierFree` (`tier-capability-gap`) — reported by `assent doctor` — plus the `.assent/**` self-edit guard (measured: `exit=0 approvals=0 merges=0 discussions=0`, `decision=BLOCK → assent-policy self-edit, no forge writes (fail-closed)`), the fork/untrusted guard, ADR-0017 §4 controlling-fact freshness, `completeForMerge()`, and the pre-write SHA guard. **Why this blocked the v0.2.0 tag:** `cli.md` and `walkthrough.md` ship with the release and go to the docs site alongside it, and the unsafe path needs no operator error — follow the walkthrough, copy its CI snippet (which omits `--arm`), harden the project until doctor reports the precondition MET, and the pipeline begins approving and merging while `cli.md` still promises it will not. Re-publishing a known-false safety guarantee under a new version is a knowing act. **Corrected surfaces:** `cmd/assent/run.go` flag help + `runRun` exit-code comment; `cmd/assent/provider_host.go` lines 34 AND 102 (a repo-wide `--arm` sweep found a second copy of "ArmEligible stays --arm ∧ APPROVE", outside the reported finding); `docs/usage/cli.md` flag row + exit-codes paragraph, plus a new *What gates approve and merge* section carrying the three preconditions, their typed refusal codes, and how to actually stay advisory (leave a precondition unmet, or `phase: observe`). `docs/usage/walkthrough.md` needed no change IN THIS LANE, and the claim is narrowed to what was actually checked: its CI snippet already omits `--arm`, and no surrounding prose reintroduces the `--arm` claim. **A second, unrelated defect was found while checking it and is fixed in this same lane under operator override — see D-135:** walkthrough line 149 says that on the REVIEW path "assent had already armed the forge's auto-merge, pinned to the evaluated commit — so the moment the last thread is resolved, **GitLab itself** merges". Nothing observed supports that. `buildDesired`'s REVIEW/BLOCK branch sets `desired.Thread` only and returns a zero `forge.Preconditions{}` — no `Approve`, no `Merge`, no `ArmEligible` — and the `forge.Forge` port has no merge-when-pipeline-succeeds/auto-merge operation at all (`Approve` and `MergeCAS` are the only write verbs; `MergeCAS` is an immediate compare-and-swap). Measured: a require-review-unproven run with `--arm` gives `exit=0 approvals=0 merges=0 discussions=1`, `decision=REVIEW arm=true → 2 forge operation(s) written`. This is a SECOND, DIFFERENT docs defect from the `--arm` one; it is recorded and corrected as D-135. **Note for the next editor:** the flag help string must contain no backquotes — `flag.PrintDefaults` reads `` `...` `` as the value-name placeholder, and a first draft rendered as `-arm assent doctor`. **Audit finding ARM-04 closed:** `cfg.arm` had zero discriminating coverage, and the finding was verified rather than accepted — on the pristine tree, mutating `summarize(result.Decision, cfg.arm, …)` to `summarize(result.Decision, true, …)` at `run.go:447` leaves the whole `cmd/assent` package `ok`. The ~30 tests that pass `--arm` do so ritually; deleting the flag would red them on flag-parse error, not behaviour. `TestRunArmFlagIsAdvisoryOnly` now pins the `arm=` summary token — the flag's ONLY remaining observable effect — in both polarities, and pins the advisory semantics itself (write outcome and the rest of the summary identical with and without the flag). Mutation-proven each direction: `…, true, …` reds `without_flag_reports_arm_false` + `flag_changes_nothing_but_the_token`; `…, false, …` reds `with_flag_reports_arm_true`. It is deliberately NOT red-first — `cfg.arm` was already threaded to `summarize`, so there was no implementation to add and mutation-kill is the honest form of the TDD obligation here. **Self-inflicted P1 caught in review and fixed before merge (finding ARM-08) — worth recording because the fix for a false safety claim introduced another one.** The *What gates approve and merge* section this row added closed with advice to stay advisory by putting the pack's rollout in `phase: observe`, "which excludes its rules from the decision structurally". That is false in the UNSAFE direction, twice over. **(i)** Structural exclusion removes the very findings that were withholding approval. Measured over `CoverWithPhaseCeiling` on an enforcing BLOCK rule: with `require: [signal]` the ceilings enforce/observe/off give BLOCK/REVIEW/REVIEW, but with **no `require:`** they give BLOCK/**APPROVE**/**APPROVE** — `observe` turns a BLOCK into an approve+merge. Only an `enforce`-phase rule marks a required obligation covered (`coverage.go:150`), so the binding's `require:` list is the sole thing holding the line — and `require` is OPTIONAL in the frozen RulesetBinding schema ("Absent or empty ⇒ no required obligations (vacuously covered)"), which is precisely the state of someone rolling out their first pack, the persona the advice targeted. The repo's own `internal/core/aggregate/phase_test.go` already asserted `DecisionApprove` for this case. All three shipped adopter packs declare non-empty `require`, so recommended configurations were safe. **(ii)** A pack's `spec.phase` is INERT unless `--pack` is passed (`run.go:247`: the ceiling is `PhaseEnforce` otherwise) — and the walkthrough's CI snippet passes no `--pack`. Measured end-to-end against a fake serving no pack route: without `--pack` the run succeeds and merges (`exit=0 approvals=1 merges=1`), proving no pack is read; with `--pack` the read happens and fails loudly. So an operator following the advice literally — edit the manifest, change nothing else — stays fully enforcing and merging while believing they are advisory: **ARM-01's exact shape, reintroduced by ARM-01's fix.** Replaced with the measured truth (see also D-135's SURF-07 note, the root cause): there is **no dry-run mode** — `assent run --dry-run` exits `2` with `flag provided but not defined: -dry-run` — no phase is a safety switch (`off` yields APPROVE too), and **the only reliable lever is leaving one of the three arming preconditions unmet.** The decision matrix above is now published in `cli.md` rather than summarised. **Explicitly NOT done, both post-tag follow-ups needing an openspec change proposal and their own red tests:** making `--arm` gate writes (contradicts `REQ-E4-S06-05`, would red `TestRunForgeProbedArmingGatesWrites`) and removing the flag (would break working v0.1.0 pipelines for no safety gain once the docs are honest). Revert: `git revert` this lane — restores the false claim and changes no behaviour, since the lane changes none. | +| D-134 | 2026-08-09 | **`--arm` is ADVISORY-ONLY and has gated nothing since `c05cde0`; this SUPERSEDES D-041's operator-facing claim "unarmed APPROVE → advisory-only, zero writes", which was true when recorded and has been false since.** D-041 is left byte-unchanged per GUIDELINES (supersede, don't edit): its evidence was gathered on 2026-07-27 against `4addc3d`, which `git merge-base --is-ancestor 4addc3d c05cde0` confirms is an ancestor of the commit that invalidated it — so the row was accurate on its own date and this one records where it stopped being accurate, not that it was wrong on arrival. **What changed:** `c05cde0` (2026-08-04, `:sparkles: feat(run): wire forge Snapshot/Resolve on assent run (E4-S06)`) replaced `Preconditions{ArmEligible: cfg.arm}` with `ArmEligible: armEligible`, the forge-probed `forge.PreconditionFromCapabilities` value, closing D-034/D-074/D-078 per `REQ-E4-S06-05`. **That engine behaviour is CORRECT and stays** — it is pinned by a genuinely discriminating test, `TestRunForgeProbedArmingGatesWrites`, which passes no `--arm` and asserts the writes happen. What went unrecorded is the operator-facing consequence: a flag documented as required-for-writes silently became cosmetic. `git show c05cde0 -- cmd/assent/run.go \| grep BoolVar` returns nothing — the commit rewrote `buildDesired`'s own arming comment and touched neither the flag's help string nor `runRun`'s doc comment. **Measured through the production entry point `runRun` against `fakeGitLab`, not transcribed:** on a forge-probe-ELIGIBLE APPROVE fixture, `args=[]` → `exit=0 approvals=1 merges=1`, summary `decision=APPROVE arm=false → 3 forge operation(s) written`; `args=[--arm=false]` → byte-identical; `args=[--arm=true]` → `exit=0 approvals=1 merges=1`. **The flag's default is `false`, so every run that never passes `--arm` was in the state the help string described as safe.** On a probe-INELIGIBLE fixture (C3 gate absent) both `args=[--arm]` and `args=[]` give `exit=0 approvals=0 merges=0`, summary `… → advisory-only (arming precondition unmet, no approve/merge)`. **The real gate** is the default-deny `internal/forge/precondition.go`: `ArmEligible` requires ALL of C17 `ProtectedPipelineExternal` (refusal `insecure-topology`), C3/ADR-0009 `DiscussionsResolvedGate` (`discussions-gate-missing`), and C6/C7 `HasApprovalRulesAPI && Tier != TierFree` (`tier-capability-gap`) — reported by `assent doctor` — plus the `.assent/**` self-edit guard (measured: `exit=0 approvals=0 merges=0 discussions=0`, `decision=BLOCK → assent-policy self-edit, no forge writes (fail-closed)`), the fork/untrusted guard, ADR-0017 §4 controlling-fact freshness, `completeForMerge()`, and the pre-write SHA guard. **Why this blocked the v0.2.0 tag:** `cli.md` and `walkthrough.md` ship with the release and go to the docs site alongside it, and the unsafe path needs no operator error — follow the walkthrough, copy its CI snippet (which omits `--arm`), harden the project until doctor reports the precondition MET, and the pipeline begins approving and merging while `cli.md` still promises it will not. Re-publishing a known-false safety guarantee under a new version is a knowing act. **Corrected surfaces:** `cmd/assent/run.go` flag help + `runRun` exit-code comment; `cmd/assent/provider_host.go` lines 34 AND 102 (a repo-wide `--arm` sweep found a second copy of "ArmEligible stays --arm ∧ APPROVE", outside the reported finding); `docs/usage/cli.md` flag row + exit-codes paragraph, plus a new *What gates approve and merge* section carrying the three preconditions, their typed refusal codes, and how to actually stay advisory. **Amended 2026-08-09 (factual-record correction, not a decision reversal — audit DOC finding):** this clause as first written read "(leave a precondition unmet, or `phase: observe`)", and the `phase: observe` half is RETRACTED ~2,000 words below as ARM-08, "false in the unsafe direction, twice over". The retraction is the truth and it is restated here, at the natural stopping point, because a reader who stops after "Corrected surfaces" was leaving with the unsafe advice: **the only reliable lever is leaving one of the three arming preconditions unmet.** `phase: observe` is NOT a safety switch — with no `require:` on the binding it turns a BLOCK into APPROVE+merge, and a pack's `spec.phase` is inert unless `--pack` is passed. See ARM-08 below for the measurements. `docs/usage/walkthrough.md` needed no change IN THIS LANE, and the claim is narrowed to what was actually checked: its CI snippet already omits `--arm`, and no surrounding prose reintroduces the `--arm` claim. **A second, unrelated defect was found while checking it and is fixed in this same lane under operator override — see D-135:** walkthrough line 149 says that on the REVIEW path "assent had already armed the forge's auto-merge, pinned to the evaluated commit — so the moment the last thread is resolved, **GitLab itself** merges". Nothing observed supports that. `buildDesired`'s REVIEW/BLOCK branch sets `desired.Thread` only and returns a zero `forge.Preconditions{}` — no `Approve`, no `Merge`, no `ArmEligible` — and the `forge.Forge` port has no merge-when-pipeline-succeeds/auto-merge operation at all (`Approve` and `MergeCAS` are the only write verbs; `MergeCAS` is an immediate compare-and-swap). Measured: a require-review-unproven run with `--arm` gives `exit=0 approvals=0 merges=0 discussions=1`, `decision=REVIEW arm=true → 2 forge operation(s) written`. This is a SECOND, DIFFERENT docs defect from the `--arm` one; it is recorded and corrected as D-135. **Note for the next editor:** the flag help string must contain no backquotes — `flag.PrintDefaults` reads `` `...` `` as the value-name placeholder, and a first draft rendered as `-arm assent doctor`. **Audit finding ARM-04 closed:** `cfg.arm` had zero discriminating coverage, and the finding was verified rather than accepted — on the pristine tree, mutating `summarize(result.Decision, cfg.arm, …)` to `summarize(result.Decision, true, …)` at `run.go:447` leaves the whole `cmd/assent` package `ok`. The ~30 tests that pass `--arm` do so ritually; deleting the flag would red them on flag-parse error, not behaviour. `TestRunArmFlagIsAdvisoryOnly` now pins the `arm=` summary token — the flag's ONLY remaining observable effect — in both polarities, and pins the advisory semantics itself (write outcome and the rest of the summary identical with and without the flag). Mutation-proven each direction: `…, true, …` reds `without_flag_reports_arm_false` + `flag_changes_nothing_but_the_token`; `…, false, …` reds `with_flag_reports_arm_true`. It is deliberately NOT red-first — `cfg.arm` was already threaded to `summarize`, so there was no implementation to add and mutation-kill is the honest form of the TDD obligation here. **Self-inflicted P1 caught in review and fixed before merge (finding ARM-08) — worth recording because the fix for a false safety claim introduced another one.** The *What gates approve and merge* section this row added closed with advice to stay advisory by putting the pack's rollout in `phase: observe`, "which excludes its rules from the decision structurally". That is false in the UNSAFE direction, twice over. **(i)** Structural exclusion removes the very findings that were withholding approval. Measured over `CoverWithPhaseCeiling` on an enforcing BLOCK rule: with `require: [signal]` the ceilings enforce/observe/off give BLOCK/REVIEW/REVIEW, but with **no `require:`** they give BLOCK/**APPROVE**/**APPROVE** — `observe` turns a BLOCK into an approve+merge. Only an `enforce`-phase rule marks a required obligation covered (`coverage.go:150`), so the binding's `require:` list is the sole thing holding the line — and `require` is OPTIONAL in the frozen RulesetBinding schema ("Absent or empty ⇒ no required obligations (vacuously covered)"), which is precisely the state of someone rolling out their first pack, the persona the advice targeted. The repo's own `internal/core/aggregate/phase_test.go` already asserted `DecisionApprove` for this case. All three shipped adopter packs declare non-empty `require`, so recommended configurations were safe. **(ii)** A pack's `spec.phase` is INERT unless `--pack` is passed (`run.go:247`: the ceiling is `PhaseEnforce` otherwise) — and the walkthrough's CI snippet passes no `--pack`. Measured end-to-end against a fake serving no pack route: without `--pack` the run succeeds and merges (`exit=0 approvals=1 merges=1`), proving no pack is read; with `--pack` the read happens and fails loudly. So an operator following the advice literally — edit the manifest, change nothing else — stays fully enforcing and merging while believing they are advisory: **ARM-01's exact shape, reintroduced by ARM-01's fix.** Replaced with the measured truth (see also D-135's SURF-07 note, the root cause): there is **no dry-run mode** — `assent run --dry-run` exits `2` with `flag provided but not defined: -dry-run` — no phase is a safety switch (`off` yields APPROVE too), and **the only reliable lever is leaving one of the three arming preconditions unmet.** The decision matrix above is now published in `cli.md` rather than summarised. **Explicitly NOT done, both post-tag follow-ups needing an openspec change proposal and their own red tests:** making `--arm` gate writes (contradicts `REQ-E4-S06-05`, would red `TestRunForgeProbedArmingGatesWrites`) and removing the flag (would break working v0.1.0 pipelines for no safety gain once the docs are honest). Revert: `git revert` this lane — restores the false claim and changes no behaviour, since the lane changes none. | | D-135 | 2026-08-09 | **The deferred forge auto-merge in ADR-0009's challenge-resolution amendment was never implemented, and `docs/usage/walkthrough.md` shipped it as fact — corrected to the measured behaviour; the ADR is left to its own amendment.** `walkthrough.md` line 149 read: *"They resolve the thread ('intentional, ticket TOPIC-123'). assent had already armed the forge's auto-merge, pinned to the evaluated commit — so the moment the last thread is resolved, **GitLab itself** merges (ADR-0009 amendment)."* **This is implementation-vs-design DRIFT, not a transcription slip.** The walkthrough was faithfully describing [ADR-0009](../adr/0009-execution-modes.md)'s Amendment (2026-07-21, F2/F3), which specifies that on `challenge` findings assent "approves conditionally and **arms forge auto-merge pinned to the evaluated SHA** (GitLab: 'merge when pipeline succeeds' + all-discussions-resolved merge gate)". That mechanism does not exist in the code. **Verified three independent ways, not reasoned:** (1) `buildDesired`'s REVIEW/BLOCK branch sets `desired.Thread` only and returns a zero-valued `forge.Preconditions{}` — no `Approve`, no `Merge`, no `ArmEligible`; (2) the `forge.Forge` port has **no** deferred-merge verb at all — `Approve` and `MergeCAS` are the only write verbs, and `MergeCAS` issues an immediate `PUT /merge?sha=`, with a repo-wide grep for `merge_when` / `auto_merge` / `when_pipeline` over `internal/` + `cmd/` returning nothing outside `AutoMergeEligible`, a doctor-report field that D-134's own note confirms is never consulted for arming; (3) `Thread.Resolved` is read ONLY inside `internal/forge`'s thread-idempotence logic, never by `internal/core` — so thread resolution is not evidence and cannot move a decision. **Measured end-to-end through `runRun` against `fakeGitLab`, replaying the walkthrough's own narrative** (require-review unproven → REVIEW): run 1 → `exit=0 threads=1 notesPosted=1 approvals=0 merges=0`, `decision=REVIEW arm=false → 2 forge operation(s) written`; the contributor then resolves every thread; run 2 → `exit=0 threads=1 notesPosted=1 notesUpdated=1 approvals=0 merges=0`, decision **REVIEW, unchanged**. The MR never merges. Rerun idempotence holds (no duplicate thread; the summary comment is updated in place). **Direction of the error, stated plainly because it differs from D-134's:** this one OVER-states automation. Its risk direction is fail-SAFE — no unintended write can follow from it, which is the opposite of D-134/ARM-01, where the docs under-stated what gets written to other people's merge requests. It is nonetheless **false**, not merely optimistic: the described mechanism is absent from the codebase, and the failure mode is an operator or contributor waiting indefinitely for a merge that will never arrive, with no diagnostic anywhere telling them why. **Severity, corrected from this row's first draft (review finding D135-CAL):** same defect CLASS as D-134 — a false claim about tool behaviour in documentation that ships with the release — but NOT tag-blocking on the same reasoning, which the first draft asserted and which overstated it. D-134/ARM-01 blocked the tag because its false claim ran in the **unsafe** direction on the primary surface: the docs under-stated what gets written to other people's merge requests. This one runs the other way — it over-states automation, and it fails loudly and recoverably (someone waits for a merge that does not come). Standing alone it is **P2**. Fixing it in this lane was still right, on operator override: it was known, it was in a file already open for exactly this reason, and shipping a defect we had found and chosen not to fix is its own problem. But it did not independently block the tag, and this row no longer claims it did. **Corrected:** walkthrough Step 6's prose now states what was measured (thread + summary comment, no approve, no merge, nothing armed; resolving satisfies GitLab's C3 merge gate but is not assent evidence; forges do not start a pipeline on resolution, per ADR-0009's own text; the merge happens only on a LATER run that reaches APPROVE on its own inputs, performed by assent immediately and SHA-pinned), and the step's `> **Shipped**` banner now names the unimplemented arming as an explicit exception — preserving the DOC-09 pin, which requires each `## Step` to be followed by a `Shipped`/`Planned` banner. A short *What this means in practice* callout closes the operator loop: an MR parked on a `challenge` finding does not merge when the last thread is resolved. **[ADR-0009](../adr/0009-execution-modes.md): annotated, NOT decided.** The ADR ships with the docs site — `mkdocs.yml`'s nav carries `ADRs: adr/README.md`, whose index links every ADR page — so it was, until this lane, a published document specifying a mechanism the tool does not have, with a Status caveat (`Accepted (partial: one-shot arming restrictions per ADR-0017 §4; P2-E5)`) that does not name the gap. Its Amendment now carries an **Implementation status** note recording that point 1's deferred forge auto-merge is unimplemented, citing this row, and naming the two honest resolutions as OPEN OPTIONS without picking one: (a) build the deferred arming (GitLab C11 — `PUT .../merge` with `auto_merge=true` + `sha=`), or (b) retract the amendment and re-derive the challenge-resolution story around the immediate merge that exists. **The amendment's normative text is untouched and the decision stays open** — recording that a specified mechanism is unbuilt is a verifiable statement of fact (AGENTS.md rule 6); choosing what to do about it is the architecture decision, and it is the operator's. The annotation went in the amendment BODY rather than the Status row by choice, not by constraint: probed empirically by appending to Status and re-running `hack/docs/truthlag_pins_test.sh`, which still reported `PASS ADR index: 20 ADR status rows agree with their files` — the pin compares only the FIRST whitespace token of the Status cell against the index's status column, so `Accepted …` stays green however much follows. It is nonetheless the wrong home: Status is the ADR LIFECYCLE field the index renders, an implementation gap is a different axis, and trailing free-text there would never surface on the index page a reader actually scans. **Open item for the next architecture pass: ADR-0009's Amendment describes behaviour the tool does not have — see the Implementation status note there, which points back here.** **Third-surface sweep, done because the fix made the word "armed" load-bearing:** `grep -i "merge when|when pipeline|pipeline succeeds|forge merges|GitLab itself|when every thread"` over `docs/`, `README.md` and `cmd/` finds NO user-facing string promising DEFERRED arming. Every product-doc and CLI use of "arm"/"auto-merge" maps to assent's own `ArmEligible` — permission to perform the IMMEDIATE approve+merge, which does exist: `assent doctor` prints `arming precondition MET — forge-probed, auto-merge may be armed` / `advisory-only — auto-merge NOT armed`, and README/`docs/index.md`/`docs/vision.md` use "auto-merge" as the product category (assent merges automatically), not as the forge's deferred feature. The only surviving deferred-arming text is ADR-0009's amendment (left to its own amendment, above) and the two `docs/planning/forge-dossier-*.md` research notes, which are OUT of the mkdocs nav per GUIDELINES and correctly describe what the forge CAN do (GitLab C11) rather than what assent does. **One clarification was still added** to `docs/usage/cli.md`'s `assent doctor` section — a surface this lane edited — stating that "armed" means assent may approve and merge immediately and SHA-pinned, NOT that it hands the forge a deferred merge-when-pipeline-succeeds instruction. The doctor SYNOPSIS line was deliberately not touched: it is pinned verbatim by `TestCLIDocCoversSubcommands` against `main.go`'s dispatch table, so rewording it is a CLI change, not a docs change. **Second annotation in the same ADR (review finding SURF-07), and the ROOT CAUSE of the ARM-08 defect recorded in D-134:** ADR-0009's mode table specifies six entrypoints and only `run` exists. There is **no `--dry-run` flag** — measured on the built binary, `assent run --dry-run …` exits `2` with `flag provided but not defined: -dry-run`, and a repo-wide search for `dry.run` across `cmd/` and `internal/` returns nothing; `explain`, `serve`, `scan` and `stats` are likewise undispatched and exit `2` against the shipped table (`run doctor lint test compare catalogue render eval-input version help`). So the ADR's *"dry-run swaps in a recorder"* and its Consequence *"Every doc example can show the dry-run first — the adoption path starts with zero risk"* are both untrue of the tool. **This is why the CLI reference invented `phase: observe` as the advisory lever: the sanctioned mechanism was never built, so the docs substituted one — and the substitute was unsafe.** Annotated with the same treatment and the same restraint as the amendment (fact recorded, normative text untouched, options named, neither picked), following the precedent the walkthrough already set with "**Planned — `assent explain` does not exist**". The ADR's **Status** row was also extended (review finding ADR9-ST) to say `Substantially unimplemented as of 2026-08-09`, so a reader scanning `docs/adr/README.md` sees the gap — free, because the index pin reads only the first whitespace token, as probed above. **Related open question, deliberately NOT fixed here (review finding SURF-08):** [OQ-29](../planning/open-questions.md) records that `PolicyProfile.spec.writes: false` is a frozen-schema field documented as an *architectural invariant* (`writes:false` "Never calls `Reconcile`") with **no runtime enforcement whatsoever** — `cmd/assent/run.go` never loads a profile — while the `single-writer-profile` lint hard error compels adopters to author the field. That needs an operator ruling, not a docs edit. Revert: restore the two walkthrough paragraphs and the two ADR annotations — reinstates false automation claims, changes no behaviour. | | D-136 | 2026-08-09 | **Merge commits are skipped STRUCTURALLY by `cliff.toml`; SUPERSEDES the load-bearing status of D-125's "prefix every merge subject" habit (D-125 otherwise stands in full).** `cliff.toml` sets `conventional_commits = false` + `filter_unconventional = false` and ends its parser list with a catch-all `{ message = ".*", group = "Other" }`, so any subject not explicitly skipped renders — merge commits included. Measured on `origin/main` @ `7513d79`: three `Merge remote-tracking branch 'origin/main' into lane/…` lines in the **Unreleased** section and three more inside `[0.1.0]`, and because `release.yaml` renders the GitHub Release body with `git-cliff --latest` and deliberately no `--strip header` (D-125/AUD-S02 §6), the unreleased three would have shipped verbatim onto the v0.2.0 Release page. The only defence was a working rule — prefix every merge subject with one of the two cliff-skipped forms — which depends on each integrator remembering and had already failed twice in the unreleased section alone. **Fix:** one commit parser, first in the list, keyed on the commit SHAPE rather than its text: `{ field = "merge_commit", pattern = "true", skip = true }` (`merge_commit` is true iff the commit has more than one parent). Chosen over a `^Merge ` message rule because it holds for a merge whose author wrote any subject at all, and over touching the catch-all because that entry is load-bearing for D-125's drift gate. **What of D-125 still stands:** the `pull_request` guard on the CI changelog step (see below), the Keep-a-Changelog headings, the D-124 `check:` wiring closure, and — still LOAD-BEARING, not belt-and-braces — the rule that a *regeneration* commit must be subject-prefixed `:memo: chore(release):` or `:wrench: chore(release):`, since a regeneration commit is an ordinary single-parent commit, and the rule to regenerate after the last content commit and after any `git merge origin/main` (a merge changes which commits are in range even though the merge itself no longer renders). What is downgraded to belt-and-braces is only the habit of prefixing MERGE subjects. **Premise retired, NOT replaced — see OQ-30.** D-125 justified `if: github.event_name != 'pull_request'` by "the PR merge ref's synthetic merge subject always renders, so no committed `CHANGELOG.md` can match". That reason is **dead**: this row's parser skips exactly that commit. A successor reason was drafted for this row — "the merge ref also carries every commit landed on `main` since the branch forked, so the render is a union the branch's file cannot match" — and then measured, and it **measures false** (three measurements in OQ-30; the direct counterexample is this lane's own head merged into a `main` that HAD moved: `verify-changelog: ok`, zero diff lines). The mechanism it overlooked is that `CHANGELOG.md` is itself three-way merged, so the merge ref's copy already contains the base's lines; base movement therefore ends in a clean merge that matches, or in a `CHANGELOG.md` conflict that leaves the PR unmergeable and produces no merge ref at all. **So the guard is retained WITHOUT a demonstrated reason, deliberately: removing it is a change to CI gating that needs its own evidence, and this lane's mandate is the changelog rendering, not the gate's placement.** Whether a PR-scoped changelog gate is now viable is UNTESTED and recorded as **OQ-30**, with the three measurements and a fourth found while writing them up (merge DIRECTION decides the render order: merging `main` INTO a lane reds `verify-changelog` on ordering alone, while the base-first direction GitHub actually mints matched), so the next person starts from data. The four sites that asserted the dead premise (`Taskfile.yml` `check:`, `verify.yaml`'s step, `hack/release/README.md`, `changelog_gate_test.sh` §3's fail message) now say only that, and point at OQ-30. Writing a second unmeasured justification into the very commit that corrects the first one is the defect this paragraph exists to avoid. **Accepted cost:** a merge subject can no longer carry changelog content — the merged branch's own commits still render, so nothing is lost, but an integrator who writes `:sparkles: feat(x): …` on a merge commit will not see it in the notes. **Regeneration consequence:** `task changelog-write` also drops the three merge lines from the already-published `[0.1.0]` section. Accepted: `CHANGELOG.md` is regenerated in full from history on every run and has never been append-only, 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` §6 asserts the release body rendered with release.yaml's OWN extracted arguments carries no merge subject; §7 asserts the same over real history and re-renders with the parser entry deleted (mutation proven to have landed) to show the lines return and that the clean/mutant renders differ by nothing else; §7b builds a sandbox repo and shows git's default `Merge branch 'x'`, the `Merge into ` CI shape and a hand-written conventional merge subject are all skipped while five ordinary commits render. Revert: drop the `merge_commit` parser entry and §6's/§7's assertions — reopens the Release-page leak and restores full dependence on integrator memory. | | 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. | diff --git a/docs/vision.md b/docs/vision.md index 750ccf0..6d5f3fe 100644 --- a/docs/vision.md +++ b/docs/vision.md @@ -48,8 +48,10 @@ always produce the same decision. No LLM in the decision path. ### Modes -The same pipeline runs as: a **CI job** (primary) and a **local dry-run** ("what would the -gate say?"). Three further modes are *planned*, not shipped — **explain** (full per-rule +One mode ships today: the **CI job** (primary) — `assent run`. Four further modes are +*planned*, not shipped — a **local dry-run** ("what would the gate say?"; there is no +`--dry-run` flag today, and passing one exits `2` — run `assent test` against +`.assent/tests/**` for the local feedback loop instead), **explain** (full per-rule trace; today the emitted `DecisionRecord` carries the same information), a **historical scan** over past MRs feeding `stats` (backtesting a policy before trusting it — no database, just report artifacts; `assent compare` covers the corpus-replay case today), and a From c3451d2be796af1fcc1bbfc5838602fc191f1a59 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:58:41 +0200 Subject: [PATCH 6/7] :memo: docs: record the reliability + security deferrals (D-138, D-139, OQ-31); OQ-29 to P1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md rule 6 — the audit's reliability and security lenses ruled six P1/P2 findings not blocking for v0.2.0, so the deferral gets written down rather than carried in a session. D-138 (RELI-01/02/03): all pre-existing at v0.1.0, verified at 1d8aa60; the machine direction holds — no probed path merges anything unevaluated — and the failure is confined to the human signal channel. Records that D-130's compensating control does NOT reach the GUARD-1 self-edit path, because no thread is posted there, so nothing holds the merge and no later run repairs the banner; that RELI-01's fix reopens a frozen acceptance criterion and so is out of release scope by construction; and that RELI-02 is unrecoverable without a human, with the race as the declared default posture. Also folds in the ADR-0019 `duplicate_prevention:` annotate-don't-emit call, same human-signal shape. D-139 (SEC-01/04/05) with the named fixes so the next lane is unambiguous, the discharged condition that the release must not read as though `--checkout` has been made safe, and the SEC-05 x SEC-03 composition recorded explicitly AS A HYPOTHESIS — no evidence real GitLab omits source_project_id. The lens withdrew its SEC-08 as non-novel (D-130 covers it); not written up. OQ-31: may the GUARD-1 BLOCK path write a summary or supersession note, and if not, what channel carries BLOCK to a human given no thread and exit 0? Undecided. RELI-03 gets no OQ — D11 already decided it; it is unbuilt, not undecided. OQ-29 raised to P1: its escalation condition ("if docs/architecture/ ever enters the nav") was already true when written. MkDocs publishes every file in docs_dir regardless of nav — measured, architecture/policy-profiles/ returns 200, the sitemap carries 63 locs against ~10 nav entries, and the false invariant is in search_index.json. GUIDELINES 57-58 is read as a publication boundary but creates only a nav boundary. Plus the RELI-03 comment at internal/forge/forge.go, corrected in place: it cited reset_approvals_on_push as its safety argument, a setting that appears in zero Go files, and deferred to P4-E1-S10, a slice that shipped. Text only, no behaviour change — an asserted-and-unbuilt mitigation terminates the search, which is how this survived two prior audits. --- docs/decisions/decisions.md | 2 ++ docs/planning/open-questions.md | 3 ++- internal/forge/forge.go | 13 +++++++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/decisions/decisions.md b/docs/decisions/decisions.md index 323e7f0..2b407ce 100644 --- a/docs/decisions/decisions.md +++ b/docs/decisions/decisions.md @@ -138,3 +138,5 @@ project/process decisions. | D-135 | 2026-08-09 | **The deferred forge auto-merge in ADR-0009's challenge-resolution amendment was never implemented, and `docs/usage/walkthrough.md` shipped it as fact — corrected to the measured behaviour; the ADR is left to its own amendment.** `walkthrough.md` line 149 read: *"They resolve the thread ('intentional, ticket TOPIC-123'). assent had already armed the forge's auto-merge, pinned to the evaluated commit — so the moment the last thread is resolved, **GitLab itself** merges (ADR-0009 amendment)."* **This is implementation-vs-design DRIFT, not a transcription slip.** The walkthrough was faithfully describing [ADR-0009](../adr/0009-execution-modes.md)'s Amendment (2026-07-21, F2/F3), which specifies that on `challenge` findings assent "approves conditionally and **arms forge auto-merge pinned to the evaluated SHA** (GitLab: 'merge when pipeline succeeds' + all-discussions-resolved merge gate)". That mechanism does not exist in the code. **Verified three independent ways, not reasoned:** (1) `buildDesired`'s REVIEW/BLOCK branch sets `desired.Thread` only and returns a zero-valued `forge.Preconditions{}` — no `Approve`, no `Merge`, no `ArmEligible`; (2) the `forge.Forge` port has **no** deferred-merge verb at all — `Approve` and `MergeCAS` are the only write verbs, and `MergeCAS` issues an immediate `PUT /merge?sha=`, with a repo-wide grep for `merge_when` / `auto_merge` / `when_pipeline` over `internal/` + `cmd/` returning nothing outside `AutoMergeEligible`, a doctor-report field that D-134's own note confirms is never consulted for arming; (3) `Thread.Resolved` is read ONLY inside `internal/forge`'s thread-idempotence logic, never by `internal/core` — so thread resolution is not evidence and cannot move a decision. **Measured end-to-end through `runRun` against `fakeGitLab`, replaying the walkthrough's own narrative** (require-review unproven → REVIEW): run 1 → `exit=0 threads=1 notesPosted=1 approvals=0 merges=0`, `decision=REVIEW arm=false → 2 forge operation(s) written`; the contributor then resolves every thread; run 2 → `exit=0 threads=1 notesPosted=1 notesUpdated=1 approvals=0 merges=0`, decision **REVIEW, unchanged**. The MR never merges. Rerun idempotence holds (no duplicate thread; the summary comment is updated in place). **Direction of the error, stated plainly because it differs from D-134's:** this one OVER-states automation. Its risk direction is fail-SAFE — no unintended write can follow from it, which is the opposite of D-134/ARM-01, where the docs under-stated what gets written to other people's merge requests. It is nonetheless **false**, not merely optimistic: the described mechanism is absent from the codebase, and the failure mode is an operator or contributor waiting indefinitely for a merge that will never arrive, with no diagnostic anywhere telling them why. **Severity, corrected from this row's first draft (review finding D135-CAL):** same defect CLASS as D-134 — a false claim about tool behaviour in documentation that ships with the release — but NOT tag-blocking on the same reasoning, which the first draft asserted and which overstated it. D-134/ARM-01 blocked the tag because its false claim ran in the **unsafe** direction on the primary surface: the docs under-stated what gets written to other people's merge requests. This one runs the other way — it over-states automation, and it fails loudly and recoverably (someone waits for a merge that does not come). Standing alone it is **P2**. Fixing it in this lane was still right, on operator override: it was known, it was in a file already open for exactly this reason, and shipping a defect we had found and chosen not to fix is its own problem. But it did not independently block the tag, and this row no longer claims it did. **Corrected:** walkthrough Step 6's prose now states what was measured (thread + summary comment, no approve, no merge, nothing armed; resolving satisfies GitLab's C3 merge gate but is not assent evidence; forges do not start a pipeline on resolution, per ADR-0009's own text; the merge happens only on a LATER run that reaches APPROVE on its own inputs, performed by assent immediately and SHA-pinned), and the step's `> **Shipped**` banner now names the unimplemented arming as an explicit exception — preserving the DOC-09 pin, which requires each `## Step` to be followed by a `Shipped`/`Planned` banner. A short *What this means in practice* callout closes the operator loop: an MR parked on a `challenge` finding does not merge when the last thread is resolved. **[ADR-0009](../adr/0009-execution-modes.md): annotated, NOT decided.** The ADR ships with the docs site — `mkdocs.yml`'s nav carries `ADRs: adr/README.md`, whose index links every ADR page — so it was, until this lane, a published document specifying a mechanism the tool does not have, with a Status caveat (`Accepted (partial: one-shot arming restrictions per ADR-0017 §4; P2-E5)`) that does not name the gap. Its Amendment now carries an **Implementation status** note recording that point 1's deferred forge auto-merge is unimplemented, citing this row, and naming the two honest resolutions as OPEN OPTIONS without picking one: (a) build the deferred arming (GitLab C11 — `PUT .../merge` with `auto_merge=true` + `sha=`), or (b) retract the amendment and re-derive the challenge-resolution story around the immediate merge that exists. **The amendment's normative text is untouched and the decision stays open** — recording that a specified mechanism is unbuilt is a verifiable statement of fact (AGENTS.md rule 6); choosing what to do about it is the architecture decision, and it is the operator's. The annotation went in the amendment BODY rather than the Status row by choice, not by constraint: probed empirically by appending to Status and re-running `hack/docs/truthlag_pins_test.sh`, which still reported `PASS ADR index: 20 ADR status rows agree with their files` — the pin compares only the FIRST whitespace token of the Status cell against the index's status column, so `Accepted …` stays green however much follows. It is nonetheless the wrong home: Status is the ADR LIFECYCLE field the index renders, an implementation gap is a different axis, and trailing free-text there would never surface on the index page a reader actually scans. **Open item for the next architecture pass: ADR-0009's Amendment describes behaviour the tool does not have — see the Implementation status note there, which points back here.** **Third-surface sweep, done because the fix made the word "armed" load-bearing:** `grep -i "merge when|when pipeline|pipeline succeeds|forge merges|GitLab itself|when every thread"` over `docs/`, `README.md` and `cmd/` finds NO user-facing string promising DEFERRED arming. Every product-doc and CLI use of "arm"/"auto-merge" maps to assent's own `ArmEligible` — permission to perform the IMMEDIATE approve+merge, which does exist: `assent doctor` prints `arming precondition MET — forge-probed, auto-merge may be armed` / `advisory-only — auto-merge NOT armed`, and README/`docs/index.md`/`docs/vision.md` use "auto-merge" as the product category (assent merges automatically), not as the forge's deferred feature. The only surviving deferred-arming text is ADR-0009's amendment (left to its own amendment, above) and the two `docs/planning/forge-dossier-*.md` research notes, which are OUT of the mkdocs nav per GUIDELINES and correctly describe what the forge CAN do (GitLab C11) rather than what assent does. **One clarification was still added** to `docs/usage/cli.md`'s `assent doctor` section — a surface this lane edited — stating that "armed" means assent may approve and merge immediately and SHA-pinned, NOT that it hands the forge a deferred merge-when-pipeline-succeeds instruction. The doctor SYNOPSIS line was deliberately not touched: it is pinned verbatim by `TestCLIDocCoversSubcommands` against `main.go`'s dispatch table, so rewording it is a CLI change, not a docs change. **Second annotation in the same ADR (review finding SURF-07), and the ROOT CAUSE of the ARM-08 defect recorded in D-134:** ADR-0009's mode table specifies six entrypoints and only `run` exists. There is **no `--dry-run` flag** — measured on the built binary, `assent run --dry-run …` exits `2` with `flag provided but not defined: -dry-run`, and a repo-wide search for `dry.run` across `cmd/` and `internal/` returns nothing; `explain`, `serve`, `scan` and `stats` are likewise undispatched and exit `2` against the shipped table (`run doctor lint test compare catalogue render eval-input version help`). So the ADR's *"dry-run swaps in a recorder"* and its Consequence *"Every doc example can show the dry-run first — the adoption path starts with zero risk"* are both untrue of the tool. **This is why the CLI reference invented `phase: observe` as the advisory lever: the sanctioned mechanism was never built, so the docs substituted one — and the substitute was unsafe.** Annotated with the same treatment and the same restraint as the amendment (fact recorded, normative text untouched, options named, neither picked), following the precedent the walkthrough already set with "**Planned — `assent explain` does not exist**". The ADR's **Status** row was also extended (review finding ADR9-ST) to say `Substantially unimplemented as of 2026-08-09`, so a reader scanning `docs/adr/README.md` sees the gap — free, because the index pin reads only the first whitespace token, as probed above. **Related open question, deliberately NOT fixed here (review finding SURF-08):** [OQ-29](../planning/open-questions.md) records that `PolicyProfile.spec.writes: false` is a frozen-schema field documented as an *architectural invariant* (`writes:false` "Never calls `Reconcile`") with **no runtime enforcement whatsoever** — `cmd/assent/run.go` never loads a profile — while the `single-writer-profile` lint hard error compels adopters to author the field. That needs an operator ruling, not a docs edit. Revert: restore the two walkthrough paragraphs and the two ADR annotations — reinstates false automation claims, changes no behaviour. | | D-136 | 2026-08-09 | **Merge commits are skipped STRUCTURALLY by `cliff.toml`; SUPERSEDES the load-bearing status of D-125's "prefix every merge subject" habit (D-125 otherwise stands in full).** `cliff.toml` sets `conventional_commits = false` + `filter_unconventional = false` and ends its parser list with a catch-all `{ message = ".*", group = "Other" }`, so any subject not explicitly skipped renders — merge commits included. Measured on `origin/main` @ `7513d79`: three `Merge remote-tracking branch 'origin/main' into lane/…` lines in the **Unreleased** section and three more inside `[0.1.0]`, and because `release.yaml` renders the GitHub Release body with `git-cliff --latest` and deliberately no `--strip header` (D-125/AUD-S02 §6), the unreleased three would have shipped verbatim onto the v0.2.0 Release page. The only defence was a working rule — prefix every merge subject with one of the two cliff-skipped forms — which depends on each integrator remembering and had already failed twice in the unreleased section alone. **Fix:** one commit parser, first in the list, keyed on the commit SHAPE rather than its text: `{ field = "merge_commit", pattern = "true", skip = true }` (`merge_commit` is true iff the commit has more than one parent). Chosen over a `^Merge ` message rule because it holds for a merge whose author wrote any subject at all, and over touching the catch-all because that entry is load-bearing for D-125's drift gate. **What of D-125 still stands:** the `pull_request` guard on the CI changelog step (see below), the Keep-a-Changelog headings, the D-124 `check:` wiring closure, and — still LOAD-BEARING, not belt-and-braces — the rule that a *regeneration* commit must be subject-prefixed `:memo: chore(release):` or `:wrench: chore(release):`, since a regeneration commit is an ordinary single-parent commit, and the rule to regenerate after the last content commit and after any `git merge origin/main` (a merge changes which commits are in range even though the merge itself no longer renders). What is downgraded to belt-and-braces is only the habit of prefixing MERGE subjects. **Premise retired, NOT replaced — see OQ-30.** D-125 justified `if: github.event_name != 'pull_request'` by "the PR merge ref's synthetic merge subject always renders, so no committed `CHANGELOG.md` can match". That reason is **dead**: this row's parser skips exactly that commit. A successor reason was drafted for this row — "the merge ref also carries every commit landed on `main` since the branch forked, so the render is a union the branch's file cannot match" — and then measured, and it **measures false** (three measurements in OQ-30; the direct counterexample is this lane's own head merged into a `main` that HAD moved: `verify-changelog: ok`, zero diff lines). The mechanism it overlooked is that `CHANGELOG.md` is itself three-way merged, so the merge ref's copy already contains the base's lines; base movement therefore ends in a clean merge that matches, or in a `CHANGELOG.md` conflict that leaves the PR unmergeable and produces no merge ref at all. **So the guard is retained WITHOUT a demonstrated reason, deliberately: removing it is a change to CI gating that needs its own evidence, and this lane's mandate is the changelog rendering, not the gate's placement.** Whether a PR-scoped changelog gate is now viable is UNTESTED and recorded as **OQ-30**, with the three measurements and a fourth found while writing them up (merge DIRECTION decides the render order: merging `main` INTO a lane reds `verify-changelog` on ordering alone, while the base-first direction GitHub actually mints matched), so the next person starts from data. The four sites that asserted the dead premise (`Taskfile.yml` `check:`, `verify.yaml`'s step, `hack/release/README.md`, `changelog_gate_test.sh` §3's fail message) now say only that, and point at OQ-30. Writing a second unmeasured justification into the very commit that corrects the first one is the defect this paragraph exists to avoid. **Accepted cost:** a merge subject can no longer carry changelog content — the merged branch's own commits still render, so nothing is lost, but an integrator who writes `:sparkles: feat(x): …` on a merge commit will not see it in the notes. **Regeneration consequence:** `task changelog-write` also drops the three merge lines from the already-published `[0.1.0]` section. Accepted: `CHANGELOG.md` is regenerated in full from history on every run and has never been append-only, 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` §6 asserts the release body rendered with release.yaml's OWN extracted arguments carries no merge subject; §7 asserts the same over real history and re-renders with the parser entry deleted (mutation proven to have landed) to show the lines return and that the clean/mutant renders differ by nothing else; §7b builds a sandbox repo and shows git's default `Merge branch 'x'`, the `Merge into ` CI shape and a hand-written conventional merge subject are all skipped while five ordinary commits render. Revert: drop the `merge_commit` parser entry and §6's/§7's assertions — reopens the Release-page leak and restores full dependence on integrator memory. | | 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. | diff --git a/docs/planning/open-questions.md b/docs/planning/open-questions.md index 08a9b5f..d64ffa0 100644 --- a/docs/planning/open-questions.md +++ b/docs/planning/open-questions.md @@ -29,5 +29,6 @@ | OQ-26 | **`assent test` `score.total` faithfulness (P5-E6-S03).** The S03 matcher computes `score.total` as Σ `finding.Points` over the enforcing `Result.Findings`, but a finding carries the AUTHORED per-firing weight `r.Points`, not `firings*r.Points` (the engine's real `pointsSum`, an intentional aggregate asymmetry, ADR-0007 Amendment 2). So for a rule that fires K>1 times the matcher UNDERcounts — safe (it can only mismatch/FAIL, never spuriously pass on higher real risk) but not faithful. A faithful total needs the engine to expose the summed `pointsSum` on `Result` (a decision-path change, its OWN fail-safety-reviewed lane — parallel to the `findings[].path` field-add, D-054(b)). Until then S03 fixtures are single-firing so `total` is exact. | E6 fast-follow / engine lane | logged by S03; leading answer: add `Result.PointsTotal` in the path/score engine lane, then Match reads it | | OQ-28 | ~~**Filesystem containment for provider reads: is PATH containment enough, or must the injected FS itself be a security boundary?** (raised P5-E5-S07/S08 while implementing `builtin/repo-file` and `builtin/resource-owner`.) The builtins clip candidates to declared `roots` with pure string guards (`cleanRel`/`underAnyRoot`) over an `os.DirFS`. Under `--checkout` that FS is the merge request's own HEAD tree — contributor-authored content — and Go documents `os.DirFS` as **not** a security boundary while `fs.Stat` follows links. Question: does the invariant "never a fact from outside the declared roots" need a syscall-level root, a per-component symlink refusal, or both?~~ **Resolved (D-129): BOTH, and they are not substitutes.** (a) `cmd/assent` injects `builtin.OpenRepoRoot` = `os.OpenRoot` + `(*os.Root).FS()`, a syscall-level boundary for every consumer of that FS; (b) `classifyCandidate` Lstats every path component and refuses any symlinked candidate — the only layer that can protect the `roots` clip, which `os.Root` cannot see. In-root symlinks are refused too; refusal is `unavailable` with a contributor-readable reason and STOPS the walk-up. Retroactive row: D-129 and `REQ-E5-S07-03` cited "OQ-28" before this table carried it (AGENTS.md rule 6 — no dangling references). | — (closed) | [decisions.md](../decisions/decisions.md) D-129/D-130; `REQ-E5-S07-03`/`REQ-E5-S08-03`. **Residual CLOSED (D-133):** `collectTree`'s silent truncation (P0) and `readIfPresent`'s governed-subject symlink (P1) are both fixed in `cmd/assent/checkout.go`. **Proof relocated — stated here so nobody re-derives it wrongly:** D-133 refuses ANY symlink under `base/`/`head/` at changed-file ENUMERATION, before providers resolve, so this row's escape is no longer reproducible end-to-end through `assent run --checkout`. The provider guard is now defence in depth, proven at `cmd/assent`'s production fact-resolution seam (`TestResolveRunFactsRefusesSymlinkedQuotaCandidate`, which pins the two layers separately) plus `internal/provider/builtin/{repo_file,resource_owner}_symlink_test.go`; it becomes the live barrier again if ADR-0008 Amendment 2's fold-the-refusal-opaque direction lands — see D-129's 2026-08-09 amendment | | OQ-16 | ~~Which **open-source repos** join the demo/test corpus?~~ **Resolved (P2-E5):** kafka/org + JulieOps descriptors + octoDNS zones, pinned by SHA with vendored excerpts — see [examples/repos/corpus.md](https://github.com/PlatformRelay/assent/blob/main/examples/repos/corpus.md) | — | [adr-acceptance-review.md](adr-acceptance-review.md); D-008/D-029 extra private shapes deferred but kept in corpus plan | -| OQ-29 | **`PolicyProfile.spec.writes: false` is a frozen-schema field with NO runtime enforcement, and lint compels adopters to author it. Operator ruling needed.** [`docs/architecture/policy-profiles.md`](../architecture/policy-profiles.md) states the recorder-only guarantee as an **"architectural invariant, not a runtime best-effort check"** — line 13: a `writes: false` profile **"Never calls `Reconcile` — no approve, merge, block, thread sync, or other forge write"**. No code enforces it, because nothing on the write path reads it. Verified by grep over non-test sources: `aggregate.ResolveProfile` and `Result.WriteAllowed` have consumers only in `internal/lint/posture.go` and inside `internal/core/aggregate` itself; `aggregate.CoverWithProfile` is called only from `internal/compare`; `policy.LoadProfile` is called only from `cmd/assent/compare.go`; and **`cmd/assent/run.go` contains the string `Profile` zero times** — it evaluates via `aggregate.CoverWithPhaseCeiling` (`run.go:533`) and reaches `buildDesired`/`forge.Reconcile` without ever loading or consulting a profile. `internal/core/aggregate/profile.go:97` documents the missing link in its own words: *"A downstream forge step reads `WriteAllowed` to know whether this run may arm/merge or is recorder-only"* — **there is no such downstream forge step.** So a `writes: false` profile does not make `assent run` recorder-only; the run behaves exactly as if no profile existed. **Why it is not merely internal:** `writes` is a REQUIRED field of the frozen `schemas/policy/v1alpha1/profile.schema.json`, whose description reads *"true = this profile authorizes forge writes for bindings in its scope; false = recorder-only"*, and the `single-writer-profile` lint hard error (`internal/lint/posture.go:83`) fails a tree where zero or more than one `writes: true` profile covers a binding — so adopters are **compelled** to author a field whose `false` value does not do what the schema says. **Severity today is P2 only because `docs/architecture/policy-profiles.md` is NOT in the mkdocs nav** (`mkdocs.yml` publishes only `c4-context.md` and `c4-container.md` under Architecture), so the invariant claim is not on the docs site. **If that directory ever enters the nav it becomes P1** — a published false safety guarantee, the D-134 shape. **Ruling needed, deliberately not taken here:** (a) implement the gate — load the covering profile on the `run` path and refuse `Reconcile` when `WriteAllowed` is false, making the documented invariant real; (b) retract the invariant language, restate `spec.writes` as comparison-scope metadata only, and say so in the schema description; or (c) accept the gap explicitly and annotate the doc, as ADR-0009 was annotated. **Not to be resolved by silently changing the frozen schema or the lint rule** — `writes` is a frozen contract field and the lint rule is load-bearing for the compare path. | Before `docs/architecture/` enters the mkdocs nav; before any release that publishes the recorder-only guarantee | Found during the D-134/D-135 docs-truth lane (review finding SURF-08). Cross-referenced from [D-135](../decisions/decisions.md). Evidence: `internal/core/aggregate/profile.go:95-101`, `internal/lint/posture.go:200-215`, `cmd/assent/run.go:533`, `schemas/policy/v1alpha1/profile.schema.json:25-28` | +| OQ-29 | **`PolicyProfile.spec.writes: false` is a frozen-schema field with NO runtime enforcement, and lint compels adopters to author it. Operator ruling needed.** [`docs/architecture/policy-profiles.md`](../architecture/policy-profiles.md) states the recorder-only guarantee as an **"architectural invariant, not a runtime best-effort check"** — line 13: a `writes: false` profile **"Never calls `Reconcile` — no approve, merge, block, thread sync, or other forge write"**. No code enforces it, because nothing on the write path reads it. Verified by grep over non-test sources: `aggregate.ResolveProfile` and `Result.WriteAllowed` have consumers only in `internal/lint/posture.go` and inside `internal/core/aggregate` itself; `aggregate.CoverWithProfile` is called only from `internal/compare`; `policy.LoadProfile` is called only from `cmd/assent/compare.go`; and **`cmd/assent/run.go` contains the string `Profile` zero times** — it evaluates via `aggregate.CoverWithPhaseCeiling` (`run.go:533`) and reaches `buildDesired`/`forge.Reconcile` without ever loading or consulting a profile. `internal/core/aggregate/profile.go:97` documents the missing link in its own words: *"A downstream forge step reads `WriteAllowed` to know whether this run may arm/merge or is recorder-only"* — **there is no such downstream forge step.** So a `writes: false` profile does not make `assent run` recorder-only; the run behaves exactly as if no profile existed. **Why it is not merely internal:** `writes` is a REQUIRED field of the frozen `schemas/policy/v1alpha1/profile.schema.json`, whose description reads *"true = this profile authorizes forge writes for bindings in its scope; false = recorder-only"*, and the `single-writer-profile` lint hard error (`internal/lint/posture.go:83`) fails a tree where zero or more than one `writes: true` profile covers a binding — so adopters are **compelled** to author a field whose `false` value does not do what the schema says. **RAISED TO P1 on 2026-08-09 — the stated escalation condition was ALREADY TRUE when it was written (audit DOC-04).** The original text read: *"Severity today is P2 only because `docs/architecture/policy-profiles.md` is NOT in the mkdocs nav, so the invariant claim is not on the docs site. If that directory ever enters the nav it becomes P1."* That rests on a false premise — **MkDocs publishes every file in `docs_dir` regardless of `nav`**; the nav controls navigation, not publication. Measured live on 2026-08-09, not reasoned: `curl -sI https://platformrelay.github.io/Assent/architecture/policy-profiles/` returns **200**; `sitemap.xml` carries **63 `` entries** against ~10 nav entries; `docs/planning/**` is fully published too; and the page's own words — the recorder-only guarantee stated as an *"architectural invariant, not a runtime best-effort check"* — are in the site's `search/search_index.json`, which indexes 420 sections and returns `architecture/policy-profiles/#write-vs-recorder-only` for that phrase. **So the published false safety guarantee is not hypothetical; it has been live the whole time, and it is searchable.** This is the D-134 shape exactly, and it is P1 by this question's own criterion. **`GUIDELINES.md`'s "docs published on the future site = product docs under `docs/` only; `docs/planning/`, `openspec/`, and agent-context stay out of the mkdocs nav" is read as a publication boundary; it creates only a NAV boundary, and nothing enforces the intended one** — a second, separate gap worth closing (an `exclude_docs`/`not_in_nav` setting, or moving non-product pages out of `docs_dir`). **Ruling needed, deliberately not taken here:** (a) implement the gate — load the covering profile on the `run` path and refuse `Reconcile` when `WriteAllowed` is false, making the documented invariant real; (b) retract the invariant language, restate `spec.writes` as comparison-scope metadata only, and say so in the schema description; or (c) accept the gap explicitly and annotate the doc, as ADR-0009 was annotated. **Not to be resolved by silently changing the frozen schema or the lint rule** — `writes` is a frozen contract field and the lint rule is load-bearing for the compare path. | **P1 — both stated conditions are already met**: the page is published (200) and indexed, and v0.1.0 already shipped the recorder-only guarantee. Needs a ruling before v0.2.1 | Found during the D-134/D-135 docs-truth lane (review finding SURF-08). Cross-referenced from [D-135](../decisions/decisions.md). Evidence: `internal/core/aggregate/profile.go:95-101`, `internal/lint/posture.go:200-215`, `cmd/assent/run.go:533`, `schemas/policy/v1alpha1/profile.schema.json:25-28` | | OQ-30 | **Is a `pull_request`-scoped CHANGELOG drift gate viable now that D-136 skips merge commits? The guard is retained with NO demonstrated reason — its original one is dead and its proposed successor measures false.** D-125 skipped the gate on `pull_request` because `refs/pull/N/merge`'s synthetic merge subject rendered into the generated changelog, so no committed `CHANGELOG.md` could match. **D-136 killed that reason** — that commit is a merge commit and is now skipped. The successor reason drafted in D-136's first version — "the merge ref also carries every commit landed on `main` since the branch forked, so the render is a union the branch's file cannot match, red by construction" — was then measured four ways and **could not be made true**: (1) PR #41's **live** `refs/pull/41/merge` (`491bb2a`, head `49eebb3` into base `7513d79`) rendered with the new `cliff.toml` → `verify-changelog: ok`, **0 diff lines**; (2) the direct counterexample — the same head merged into a `main` that **had** moved (`1d8aa60`, containing PR #40) → `verify-changelog: ok`, **0 diff lines**, i.e. not red with the base moved; (3) a synthetic sandbox where base and lane each add a commit to the **same** cliff group and each regenerate → `CONFLICT (content): Merge conflict in CHANGELOG.md`, so the PR is unmergeable, GitHub mints no merge ref, and the gate never runs. (4) **The strongest one, taken last and re-run rather than transcribed:** GitHub RE-MINTED `refs/pull/41/merge` against the moved base after all of the above. Re-fetched live — `7715bf7`, head `ee5e527` into base `1d8aa60` — and put through the real gate script: **`verify-changelog: ok`, 0 diff lines, 0 merge subjects rendered.** That is not a simulation: it is the exact artifact a `pull_request`-scoped gate would evaluate, with the base moved past the fork point AND after the lane had merged `main` in — the direction the finding below shows is hazardous — and it is green. Measurement (1)'s `491bb2a` at base `7513d79` is its stale predecessor, kept only to show the result did not depend on the base standing still. **Mechanism the dead premise overlooked:** the merge ref's `CHANGELOG.md` is not "the branch's committed file" — it is the three-way MERGE RESULT, which already contains the base's lines, because the file is merged like any other. So base movement ends in clean-and-matching or conflict-and-no-merge-ref. **The third outcome EXISTS, and merge DIRECTION decides it — measured while writing this row.** A clean textual auto-merge whose line order differs from git-cliff's topological order is red with no author error, and it reproduced immediately: merging `origin/main` **into the lane** (lane as first parent) auto-merged `CHANGELOG.md` without conflict and then failed `verify-changelog` on pure ordering — one `docs(compare)` line moved and PR #40's lines landed in a different position. The SAME two commits merged in the **merge-ref direction** (base `1d8aa60` as first parent, measurement (2) above) matched exactly. git-cliff's traversal follows parent order, so first-parent choice changes the render. This does not revive the retired premise — GitHub always mints the merge ref base-first, which is the direction that matched — but it means the clean-and-matching outcome is a property of that direction, measured on two merges, not a proof. It also re-confirms D-125's surviving rule: regenerate after any `git merge origin/main`. **Still untested:** behaviour on `pull_request_target`, on a PR from a fork, and after a force-push that re-mints the merge ref. **Counter-evidence for enabling it:** the only red reproduced on any merge ref was a branch that had not run `task changelog-write` for its own commits — a **true positive the gate exists to catch**, which argues the PR placement may now be correct rather than merely harmless. **Correction, folded in from the PR #41 review because it belongs in the row and not only in a review thread:** that review first read these greens as "the evidence points toward the PR gate being viable", and then took it back as **one measurement short**. The direction finding above supplies a false-positive mechanism it had not considered — a clean textual auto-merge whose line order differs from git-cliff's topological order reds with no author error and no author fix available. Four green measurements are therefore NOT a green light; on today's evidence the gate would not be enabled. **Ruling needed (deliberately not taken here, operator's call):** (a) enable the step on `pull_request` and delete the guard; (b) keep the guard and record the real reason once someone finds one; or (c) keep the guard permanently on cost/noise grounds and say so, rather than on a mechanism. Not to be resolved by deleting the guard on the strength of these three measurements alone — they show the claimed failure did not reproduce, not that no failure exists. | Before any change to the `pull_request` guard on the changelog step in `.github/workflows/verify.yaml`; not a release blocker — the guard is fail-safe (the gate runs locally in `task check` and on push-to-main) | Raised by the PR #41 review (finding CL-02) against D-136's first draft; measurements reproduced independently before recording. Sites now pointing here: `Taskfile.yml` `check:`, `.github/workflows/verify.yaml`, `hack/release/README.md`, `hack/release/changelog_gate_test.sh` §3. See [D-125](../decisions/decisions.md) and [D-136](../decisions/decisions.md) | +| OQ-31 | **May the GUARD-1 self-edit BLOCK path write a summary or supersession note, or is "zero forge writes on a self-modifying MR" absolute? If it is absolute, what channel carries the BLOCK to the human reviewer — given that no thread is posted and the exit code is `0`?** Raised by RELI-01 ([D-138](../decisions/decisions.md)) and deliberately left **UNDECIDED**. The tension is real in both directions. **For absolute:** `openspec/specs/p5-aud-audit-remediation/spec.md` pins *"the decision is BLOCK with zero forge writes (GUARD-1 dominance over the gap-degrade)"* as a frozen acceptance criterion, and the guard exists so that an MR editing `.assent/**` cannot make assent vouch for its own policy — any write is a write the MR's own content influenced. **Against absolute:** the only human-visible surface then keeps whatever the previous run said, which today can be `✅ Decision: APPROVE`, so the guard's output is *invisible* to the reviewer it protects, and D-130's compensating control (a REVIEW rerun upserts the summary and adds an unresolved discussion) does not reach this path because no thread is posted. Zero *authority* writes need not mean zero *communication*. **Options, none taken here:** (a) keep it absolute and carry BLOCK on a non-forge channel — a non-zero exit code, or a required CI job status; (b) permit exactly one write, a fixed-text supersession/BLOCK note with no policy-derived content, which cannot be steered by the MR; (c) permit the summary upsert but not the thread. (b) and (c) both **reopen the frozen criterion above and need an openspec change proposal first** — spec before code. Note that (a) changes an exit-code contract wrapper scripts rely on ([`docs/usage/cli.md`](../usage/cli.md)), so it is not the free option it looks like. | Before the RELI-01 fix lands (v0.2.1) | Found by the 2026-08-09 audit's reliability lens; recorded in [D-138](../decisions/decisions.md). Evidence: `cmd/assent/run.go` step-9 GUARD switch, `openspec/specs/p5-aud-audit-remediation/spec.md`, `openspec/specs/p5-e5-provider-host/spec.md` REQ-E5-S08-03 | diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 8157bf3..5484825 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -766,8 +766,17 @@ func forgeIDNum(id string) int { // the TOCTOU window BETWEEN the CurrentHeads pre-check and MergeCAS, the approval // has already been recorded, so this path can leave a DANGLING approval with NO // merge. That is deliberate and matches real GitLab (approve-then-merge?sha=, -// no rollback); the dangling approval is cleared by the forge's "remove approvals -// on new push" setting — an S10/adapter concern, not this engine slice. +// no rollback). This comment used to argue the dangling approval "is cleared by +// the forge's remove-approvals-on-new-push setting — an S10/adapter concern, not +// this engine slice". BOTH HALVES WERE FALSE and are corrected here (RELI-03, +// audit 2026-08-09, D-138): `reset_approvals_on_push` appears in ZERO Go files — +// nothing reads it, `probeCapabilities` does not fetch it, and `assent doctor` +// cannot report it — so it is an ASSUMED, never-verified property of the adopter's +// project, not a control this code relies on soundly; and the deferral pointer +// named a slice that shipped (P4-E1-S10, D-041), so the concern was never picked +// up. `spike-secure-setup.md` D11 specified refuse-to-arm when the setting is off +// and C19 specified doctor verification; neither was built. Until one of them is, +// treat the dangling approval as UNRETRACTED: `Forge` has no `Unapprove` verb. // // The P0 SAFETY invariant that ALWAYS holds is the one that matters: NO merge of // a moved/unevaluated SHA (MergeCAS re-checks all three pins atomically and fails From 47927060a23132c9a5edb0104d4a22811ed943be Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 9 Aug 2026 04:59:39 +0200 Subject: [PATCH 7/7] :memo: chore(release): regenerate CHANGELOG.md for the audit pre-tag docs lane --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46c3366..54eb0ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,10 @@ repository still gets a decision, never by following the link; no release carrie - :memo: docs: fix ARM-08 advisory advice, annotate the dry-run drift, log OQ-29 - :memo: docs(usage): say that an advisory run still writes (ADV-01) - :memo: docs(decisions): retire D-136's PR-guard premise instead of replacing it (CL-02/CL-03) +- :memo: docs(cli): record that --checkout is not bound to the evaluated commit (SEC-01) +- :memo: docs(adr): correct four ADR claims the code does not support +- :memo: docs: retract the unsafe advice at D-134's stopping point; dry-run is not shipped +- :memo: docs: record the reliability + security deferrals (D-138, D-139, OQ-31); OQ-29 to P1 ### Features - :sparkles: feat(cli): dispatch-table help listing the real subcommands (REQ-AUD-S05-01) @@ -181,6 +185,8 @@ repository still gets a decision, never by following the link; no release carrie - :bug: fix(engine): ordering a bytes operand is text ordering too — refuse it (D-131) - :bug: fix(checkout): keep a rooted --subject working, and unshadow the path package - :bug: fix(release): file gitmoji subjects by conventional type, not by emoji (REL-14, D-137) +- :bug: fix(docs): point every docs-site URL at the case-sensitive Pages path (DOC-02) +- :bug: fix(render): stop minting a 404 documentation link into every MR thread (DOC-03) ### Refactoring - :recycle: fix(forge): retry idempotent GitLab reads with bounded jittered backoff (AUD-S11, REL-04)