| OQ-1 |
~~Project name~~ Resolved: assent (D-009); repo live (D-014). Domain: assent.dev was taken (D-028); operator has since initiated a brokered purchase of assent.dev itself (GoDaddy, not yet closed). Rename SUSPENDED pending that transfer (D-149) — if it completes, the shipped apiVersion: assent.dev/v1alpha1 is already correct and no migration is needed; named fallback if it fails is assent.platformrelay.dev/v1alpha1. |
— (suspended, checkpoint at next handover) |
naming.md; D-028/D-031/D-149 |
| OQ-2 |
~~Hosting: GitHub only, or GitLab mirror (dogfooding the GitLab adapter on our own repo)?~~ Resolved (D-105): defer mirror — GitHub canonical; optional read-only GitLab mirror is operator infra, not E9 blocker. Dual-primary rejected (drift risk). No mirror workflow in E9. |
— |
E9-S11 |
| OQ-3 |
~~Two parallel frontends?~~ Resolved by ADR-0002 v2: one YAML envelope, pluggable predicate backends |
— |
superseded; successor questions: OQ-11/OQ-12 |
| OQ-4 |
~~Ship gRPC (go-plugin) tier in v1?~~ Resolved (P2-E5): defer gRPC to post-v1; HTTP/exec + builtins only (Spike C, D-012, ADR-0004 Accepted) |
— |
adr-acceptance-review.md |
| OQ-5 |
~~Policy discovery: remote packs in v1?~~ Resolved (P2-E5): local .assent/ only in v1; remote packs designed-for (ADR-0010 Accepted, D-012) |
— |
adr-acceptance-review.md |
| OQ-6 |
~~E2E default in CI: kind vs testcontainer?~~ Resolved (P2-E5 / Spike B): testcontainer in CI; kind stays for local/demo (spike-b-e2e.md — boot p50 96 s vs 126 s, ~2.4 GB vs ~3.1 GB+node, 0 flakes) |
— |
ADR-0006 Accepted |
| OQ-7 |
~~GitHub mapping for challenge~~ Resolved (P2-E5): parity for the gate, not the device — required-conversation-resolution carries acknowledgement; REQUEST_CHANGES reserved for block (forge-dossier-github.md §3). Residual live checks → Phase 5 / E10 |
Phase 5 / E10 |
ADR-0005 Accepted |
| OQ-8 |
Decision replay/audit: JSON report artifact enough, or signed/attested decision record later? |
Phase 3 |
v1: artifact (Pins in report); attestations later epic |
| OQ-9 |
~~Version pinning for reproducibility (tool digest + policy SHA in report Pins)?~~ Resolved (D-120): schemas/decision/v1alpha1/decision-record.schema.json requires toolVersion, toolDigest, policySha, sourceSha, targetSha, mergeResultDigest, factsResolvedAt. |
— |
D-120 |
| OQ-10 |
Monorepo support: multiple policy scopes per repo (path-scoped .assent/ dirs)? |
Phase 3 |
likely bindings-level path scoping |
| OQ-11 |
~~kyverno-json vs cel-go~~ Resolved (P2-E5): cel-go (ADR-0013 Accepted; Spike A) |
— |
adr-acceptance-review.md |
| OQ-12 |
~~assert authored syntax~~ Resolved (P2-E5): hybrid all/any/not trees with CEL leaves + per-leaf message (ADR-0013 Accepted) |
— |
adr-acceptance-review.md |
| OQ-13 |
~~Risk score conventions / effect escalation?~~ Resolved (P2-E5): points + per-binding thresholds only in v1; no score→effect escalation (ADR-0007 Accepted) |
— |
adr-acceptance-review.md |
| OQ-14 |
~~serve (webhook) in v1?~~ Resolved (P2-E5): v1.x / E12 post-Phase-4; architecture-ready from day 1 (ADR-0009 Accepted, D-012/D-017) |
— |
adr-acceptance-review.md |
| OQ-15 |
~~fold-to-rename opt-in?~~ Resolved (P2-E5): opt-in per class, default raw; rename never laxer than delete (ADR-0003 Accepted). Residual: similarity metric itself |
Phase 3 |
adr-acceptance-review.md |
| OQ-17 |
~~max_age default~~ Partially resolved (P2-E5): Spike C host defaults (principal/authz 1h, registry 24h, sensitive 15m; arming precondition). Schema freeze |
Phase 3 / contract fixture |
spike-c-provider.md; adr-acceptance-review.md |
| OQ-23 |
~~require-review forge mechanics~~ Leading answer (P1-E3-S02): Premium/Ultimate evidence chain = approval_rules → eligible_approvers[] → approval_state.rules[].approved_by[]; exclude MR-author and bot; never trust rule-level approved alone. Free tier: capability gap → no auto-merge for archetypes needing require-review |
P3-E1 schema slice (ApprovalEvidence per D-017) |
forge-dossier-gitlab.md §4 |
| OQ-24 |
~~Secure-setup adoption spike (topology)~~ Topology resolved (P2-E5 / P2-E4): GitLab Premium + external CI config + project access token + all-threads-resolved + merged-results + merge trains + approval rules + .assent/** human residual. North-star <1h still PENDING — operator timed clean-room run remains an open Phase-4 backlog item (do not claim confirmed). See backlog.md Phase-4 operator row. |
Phase 4 / north-star wording |
spike-secure-setup.md § North-star; timed run → HOLDS/AMEND |
| OQ-25 |
~~Success metric (roast P2-8)~~ Leading answer: independently defined routine denominator + blind holdout (labeler ≠ policy author) + ≤1% false-auto-merge budget; measure via scan/stats confusion matrix — see success-metric.md. Residual: operator adjudicates holdout labels (not invented in-tree). |
Phase 4 (adjudication) |
protocol frozen in P1-E2-S03; adjudication = operator task |
| OQ-18 |
~~GitHub arm-and-wait parity~~ Resolved (P2-E5): yes on paper with three deltas (dismissal, auto-merge revoke, merge queue) — forge-dossier-github.md §1 C8′/C11/C14, §3. Implement |
Phase 5 / E8–E10 |
ADR-0005 Accepted |
| OQ-19 |
~~Post-merge reconciliation: v1.x or out of scope?~~ In scope (D-017 B8): E12 service tier, implemented post-Phase-4 — commit↔DecisionRecord/PublicationReceipt correlation, durable safety event, optional revert MR (never direct revert) |
E12 (unlocked) |
adjudicated outcomes feed policy comparison; a human revert is evidence, not proof |
| OQ-20 |
~~Batch/sweep apply mode — or per-MR CI + serve enough?~~ In scope (D-017 B9): E12 service tier — one serialized sweep, every write through per-MR preconditions/reconciliation/budgets, no bulk bypass |
E12 (unlocked) |
scan stays recorder-only; horizontal workers unsupported until a lease exists |
| OQ-21 |
~~Per-rule rollout phases — or effect-editing sufficient?~~ Reversed (D-017 B2): explicit off/observe/enforce phase field — effect-editing loses policy identity and breaks before/after comparison; observed vs enforcing findings both recorded |
P3-E4 / ADR-0018 |
observe can never alter the enforcing decision or forge state |
| OQ-22 |
Envelope match on MR metadata: labels, draft status, author allowlists — which belong in match.mr for v1? |
Phase 3 |
draft-MRs likely skipped by default in CI template |
| 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 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 |
— |
adr-acceptance-review.md; D-008/D-029 extra private shapes deferred but kept in corpus plan |
| OQ-27 |
~~A relational CEL leaf over STRING-bound operands returns a silently WRONG boolean instead of erroring — a verified BLOCK→APPROVE flip (found by AUD-S13 / TEST-02, widened by review F4).~~ RESOLVED (D-131 / ADR-0013 Amendment 1, merged on main): an ordering operator over a text-shaped operand (string or bytes) now ERRORS — internal/core/aggregate's textOrderGuard watches every relational operand as it evaluates and refuses text in either position, on both seams (evalLeaf and the walking-skeleton evalRule) — and toCEL no longer demotes an unrepresentable numeric literal to its string form, it binds a CEL error value. Ordering raw text graduates to Rego; int()/double()/timestamp() stay the tier-1 migration path. The analysis below is retained as the record of how the defect was found and how far it reached — it describes the PRE-FIX engine. The class is any string-bound operand, not just numeric overflow. cel-go's relational operators are DEFINED over two strings (lexical compare), so they return a clean boolean where the engine's fail-safe design assumes an error. P1 instance — quoted YAML scalars, no overflow anywhere: internal/evaldecode maps a !!str literal to a Go string BY DESIGN (the differ deliberately keeps the string "12" distinct from the number 12) — but that design assumed a numeric rule over a string would fail safe, and it does not. Reproduced end-to-end through the production aggregate.Cover entry point with the D-016-shaped partitions-must-not-shrink rule (new >= old, onFailure block): partitions: 12 → 6 (numeric) yields BLOCK, 1 finding partition-count-shrunk; the identical policy and subject with partitions: "12" → "6" (quoted) yields APPROVE, ZERO findings — evalLeaf returns (true, nil), the obligation is recorded as PROVEN, and the destructive change auto-merges. Second instance — numeric overflow (the original finding): a json.Number fitting neither int64 nor float64 falls back to its string form (evaluate.go:191), so 9e399 > 1e400 evaluates true (arithmetically false). No lint guard exists: checkLeafScope and checkPredicateScope (internal/lint/scope.go) validate identifier SCOPE and checkFactsShape (facts_ref.go:249) validates facts-path shape — none type-checks relational operands, so an author gets no warning. This is the exact failure internal/evaldecode's package doc warns about ("lexically \"6\" >= \"12\" is TRUE, so a partition shrink 12->6 would be judged non-destructive and APPROVE. That is the exact forbidden outcome") — the doc believed it had closed it; it closed only the canonical-render path, not the authored !!str path. Also a docs-truth defect: evaldecode.go:61 names this "the ADR-0013 residual #1 the S02 evaluator owns" but describes it as "float64 (a lossy compare) or its string form", never saying the string form yields a silently wrong boolean rather than an error — so the residual reads as benign precision loss. Severity split, kept explicit because conflating these is how a real finding gets dismissed: the MECHANISM is P1; the over-range instance ALONE is P2 on reachability (it needs BOTH operands to exceed ~1.8e308); the P1 rests on the quoted-string case, which needs only ordinary authored YAML. Hard rule 7 (determinism) is NOT violated — a lexical compare is perfectly deterministic and reproducible. What is violated is the fail-safe direction (GUIDELINES §2 / ADR-0013: undecidable or type-mismatched must error → REVIEW, never a permissive boolean) and evaldecode's own written claim. AUD-S13 deliberately wrote NO test asserting 9e399 > 1e400 == true or the quoted-string true; blessing either would enshrine the fail-open. Candidate fixes: make a relational leaf over string-bound operands ERROR (fail-safe, preferred — a lexical compare is almost never what a policy author meant); and/or a lint hard-error when a relational operator can bind a string; and/or reject over-range numerals at the loader boundary. Rejected: big.Float (reintroduces a decision-path numeric tower). |
~~release tag BLOCKED on this~~ unblocked; severity ruling done (P1), fixed in its own decision-path lane |
found by AUD-S13 (PR #35), widened by independent review F4; NOT fixed there (tests-only lane). Fixed on main by D-131 in a dedicated decision-path lane, as required; AUD-S13's TEST-02 was realigned to the refusal contract when this lane merged main |
| OQ-29 |
RESOLVED (D-145, 2026-08-16): (a) — implement the gate. writes: false becomes runtime-enforced on the run path; a same-day doc annotation covers the gap until the code lane lands. Analysis retained below for the record. Original text: PolicyProfile.spec.writes: false is a frozen-schema field with NO runtime enforcement, and lint compels adopters to author it. docs/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 <loc> 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. 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 |
RESOLVED (D-148, 2026-08-16): (b) — keep the guard skipped on pull_request; the real mechanism (merge-direction-dependent ordering hazard) is now recorded in D-125/D-136. Analysis retained below for the record. Original text: 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 and D-136 |
| OQ-31 |
RESOLVED (D-146, 2026-08-16): (a) — "zero forge writes" stays absolute; the BLOCK is surfaced via a required CI job status reading the already-emitted DecisionRecord, not via a forge write. Analysis retained below for the record. Original text: 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) 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), 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. 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 |
| OQ-32 |
RESOLVED (D-147, 2026-08-16): (b) — add a host-side secret resolver (process env / file path / hosted store); repo-side config gains only an opaque, host-allowlisted reference name, never a literal credential or URL pairing. ADR amending ADR-0015 §7 required before code. Analysis retained below for the record. Original text: No provider transport can carry a credential, so NO provider can call Entra ID, Keycloak, or any token-authenticated IdP directly — and nothing says so. Found while designing P5-DEM (D-142). Verified across three surfaces that agree: CallHTTP (internal/provider/transport.go) sets only Content-Type: application/json — no header map, no bearer token, no client certificate; the repo-side provider schema (schemas/policy/v1alpha1/config.schema.json $defs/provider) is additionalProperties:false over exactly {type, url, failure}, so there is nowhere to put one; and ScrubEnv/ScrubArgv build the exec child's environment from scratch and refuse any name matching (?i)(TOKEN\|SECRET) even when explicitly configured, so the exec tier cannot carry one either. This is not a bug — it is ADR-0015 §7 working exactly as designed, and Spike C's TestIsolation proves it against a deliberately hostile provider that exfiltrates its whole environment and stdin. What has never been written down is the consequence: Entra ID and Keycloak both require a bearer token on every call, so the only shape that works today is a broker — a service holding the IdP credential itself, reachable by assent without one (loopback/sidecar, or mTLS terminated outside assent's transport). That is arguably the correct architecture: the credential never enters the decision path and a compromised provider's blast radius stays bounded. But it is undocumented, and it narrows what docs/vision.md:67 promises ("pluggable providers: Keycloak, LDAP, GitLab/GitHub groups, ownership files, custom plugins") and what ADR-0004 §1 planned ("OIDC/Keycloak group lookup, LDAP" as builtins — never shipped). docs/architecture/c4-context.md:19 is currently the only place stating the truth: "Keycloak / LDAP: no builtin — reachable only via the generic HTTP/exec provider transport." Ruling needed (deliberately not taken): (a) bless the broker pattern, document it in the provider-author guide, and amend docs/vision.md:67 + ADR-0004 §1 to stop implying direct IdP calls — RECOMMENDED: costs nothing, keeps ADR-0015 §7 and the isolation proof intact, and is what DEM-S02/DEM-S03 are already written against; (b) add a narrow repo-side credential channel (header or secret-ref) to the HTTP transport — reopens a frozen schema AND the trust boundary the hostile-provider isolation proof rests on, and would need its own ADR; (c) state the limitation and add nothing. Note this is not merely a docs question under (a): a reader of the vision page today would reasonably budget a Keycloak integration as "configure a builtin" and discover mid-implementation that they must also deploy and operate a broker. Not to be resolved by quietly adding a header field — that is option (b) and it is a trust-boundary change. |
Before DEM-S02 publishes the provider-author guide (the guide must state one of these answers); not a release blocker — the current behaviour is fail-safe, just undocumented |
Found designing P5-DEM (judgment call (e)); recorded in D-142. Evidence: internal/provider/transport.go CallHTTP/ScrubEnv/ScrubArgv, schemas/policy/v1alpha1/config.schema.json $defs/provider, docs/planning/spikes/spike-c-provider.md § Isolation evidence, ADR-0015 §7, ADR-0004 §1, docs/vision.md:67, docs/architecture/c4-context.md:19 |
| OQ-33 |
protected-pipeline-source (ADR-0015 §4's arming prerequisite) has no operationally decidable predicate on EITHER forge — which of three candidate routes does §4 accept as a PROBE? Today's GitLab value is a substring heuristic — strings.Contains(proj.CIConfigPath, "@") (internal/forge/gitlab/snapshot.go:292), the SEC-04 shape ADR-0021 forbids by name — which proves neither that the referenced CI config sits on a protected branch nor that the MR author cannot push to it. ADR-0015 §4 already frames the GitHub condition in composite terms and says it is "verified by assent doctor": "GitHub: workflows from the target branch (pull_request runs the base-ref workflow for forks; same-repo branches need branch protection on workflow paths)". Three candidates: (1) same-repo route — §4's own second clause: branch protection or a path-restricted ruleset over .github/workflows/** on the base branch; largely forge-readable, so the closest analogue to a GitLab-style forge-only predicate (its path-scoped-ruleset availability is unverified). (2) fork route — §4's first clause: pull_request from a fork runs the base-ref workflow with a read-only, secretless token (dossier C17); safe but advisory-only per ADR-0015 §8, so a non-arming state rather than a route to supported. (3) composite env+forge route — the trigger event is one whose definition GitHub loads from the base repo's default branch (pull_request_target / workflow_run / merge_group) and that branch requires reviews; spans cmd/assent's CI-env adapter and internal/forge, which §4 did not anticipate being asked to probe. Sub-questions: (a) does §4 accept route 1 alone as the GitHub predicate? (b) if not, does it accept the env+forge composite (route 3), or must the predicate be forge-readable only? (c) if neither, does v1 ship with arming unavailable — comment-only on GitHub, and on GitLab too once the heuristic is retired? |
E10-S04 / S09 / S11 (arming); v1 GitHub gating |
github-addressing-model.md Q2 row 11; ADR-0015 §4/§8; ADR-0021 item 3 and Consequences; audit 2026-08-09 SEC-04. Leading answer: (a) if route 1 is readable on ordinary repos, else (b) — the alternatives are a heuristic (rejected) or no gate at all. Retiring the @ heuristic is a user-visible GitLab behaviour change owed its own D-row at S04/S09 (E10 judgment call (e)) |
| OQ-34 |
Can an ADAPTER-COMPUTED CODEOWNERS eligible set ever be full under ADR-0017 §3's "typed eligible principals", and what proof would license it? §3 admits "CODEOWNERS evidence" explicitly and the frozen schema enumerates codeowners (schemas/approval/v1alpha1/approval-evidence.schema.json:40-42), so the ROUTE is legitimate — the open question is the set-equality property. GitLab's eligible_approvers[] is forge-computed; a GitHub CODEOWNERS set is adapter-computed from forge-supplied bytes, and no GitHub API returns the computed per-PR eligible code owners (dossier §2 step (b), which grades this partial). An over-permissive matcher (first- vs last-match-wins, sections, negation, case-folding, a team read returning a superset of write-holders) puts a principal into ev.Eligibility that GitHub would not accept; approvalSatisfies then records the obligation satisfied (internal/core/aggregate/approval.go:127) — harm that needs no arming, because the DecisionRecord itself asserts a governance obligation was met by an ineligible principal. GitHub's own require_code_owner_reviews is not a backstop: it enforces over the PR's changed files, while assent's eligible set is scoped to the governed subject. Candidate licences: (1) a fixture-corpus fidelity case codeowners-eligible-set-matches-forge + a positive control that reddens on a deliberately over-permissive matcher — proves fidelity to the spec, not equality with the forge; (2) a live cross-check against GitHub's own code-owner determination — scoped to changed files and mutable after PR open, so corroboration rather than authority; (3) accept unknown permanently and ship GitHub comment-only for require-review. |
E10-S08 / S14; whether GitHub can ever satisfy require-review |
github-addressing-model.md Q2 row 9; ADR-0017 §3; forge-dossier-github.md §2; ADR-0021 Consequences (which predicted exactly this unknown). Leading answer: (1), with row 9 staying unknown until the case and its positive control are green — fail-closed meanwhile, per this project's thesis |
| OQ-35 |
entry / oldEntry bind whole-entry value trees under assent test but a bare scalar under assent run — do we extend the binding to the run path, or narrow the documented contract? docs/planning/predicate-scope.md describes entry as "head-state value tree of the containing EntryRef" with no qualifier, and internal/core/aggregate/evaluate.go bindLeafActivation binds toCEL(entryOr(ch.Entry, ch.New)) — falling back to the change's scalar new/old when ch.Entry is nil. The only writer of EvalChange.Entry is internal/adoptertest/entrytree.go populateEntries (called from adoptertest.go:288); internal/evaldecode.BuildEvaluationInput — the sole production builder, reached from cmd/assent/evaldecode.go — never sets it, and cmd/ contains no reference to EntryConfig, DiffEntries or change.Entries at all (assent run calls the document-mode change.Diff via changeSetForGoverned). Consequence: a rule such as oldEntry.acls.filter(a, !(a in entry.acls)).size() == 0 passes in assent test and, in production, hits a no-such-attribute error on a scalar → predicate.error → REVIEW. The direction is fail-safe, so this is not urgent and not a release blocker; what it is not is documented, and an adopter who validates a pack with assent test has no signal that the rule will never fire in assent run. Ruling needed (deliberately not taken): (a) extend entry reconstruction to the assent run path so the two agree — the honest fix, but it puts collection-mode entry derivation on the live decision path and needs its own story; (b) state the limitation in predicate-scope.md and add an assent lint hard error for a rule that navigates entry/oldEntry as an object — cheap, keeps the contract truthful, costs adopters the capability; (c) leave as is (rejected on sight — it is a silent test/production divergence). Not to be resolved by a fixture that only runs under assent test — that is precisely the "test that cannot fail" this repo's reviews keep finding. |
Gates nothing. D-156 records that both resolutions strike the set-difference shape (extending makes CEL express it; narrowing makes it an input-availability failure a Rego module inherits unchanged), and the graph-relationship shape needs no entry tree — its adjacency arrives as a flat cardinality: set fact. What is actually at stake is a silent assent test / assent run divergence: a pack an adopter validates green can contain a rule that never fires in production. Blocks nothing today. |
Found writing the tier-1 ceiling record (E11-S01); recorded in D-156. Evidence: internal/core/aggregate/evaluate.go (bindLeafActivation, entryOr), internal/adoptertest/entrytree.go, internal/evaldecode/evaldecode.go BuildEvaluationInput, cmd/assent/run.go:293 changeSetForGoverned, docs/planning/predicate-scope.md |
| OQ-36 |
The frozen provider declaration has no object/map type, yet the authoring surface and builtin/repo-file together permit a mapping-valued fact and dynamic navigation into it — is a mapping-shaped fact value in-contract or out? schemas/provider/v1alpha1/response.schema.json freezes declaration.type to boolean \| string \| integer \| principal and cardinality to single \| set; value itself carries no JSON-Schema type constraint ("shape governed by declaration.type/cardinality") and provider.ResolveFactsChecked cross-checks the declaration, never the value. Meanwhile builtin/repo-file maps each requested output to a top-level key of the resolved file via readMapping (map[string]any from yaml.Unmarshal), so a top-level key holding a mapping is emitted verbatim; factsToCEL/toCEL bind it as a CEL map; and internal/lint/facts_ref.go's D-051 shape check permits arbitrary navigation past .value (selectChainFields stops the chain at an index), so facts.registry.topics.value[string(new)].retentionMs is lint-clean and compiles. So a keyed cross-manifest join works today under a declaration that cannot describe it. This is not hypothetical polish: D-156 struck the cross-manifest shape from E11's scope partly on the strength of that spelling, and it is the one leg of that strike resting on an undeclarable value. Ruling needed (deliberately not taken): (a) add object (or map) to the declaration type enum — an announced additive change to a frozen schema, needs its own openspec change and API_STABILITY.md entry, and widens what a hostile provider may inject into the decision path; (b) state that a fact value must match its declared scalar/set shape and enforce it host-side in ResolveFactsChecked (a mismatch → invalid, value dropped) — fail-safe and closes the gap, but it breaks the keyed-join spelling above and returns the cross-manifest sub-shape B2 to the ceiling; (c) document the status quo as intentional — the declaration describes the leaf type and navigation into a container is the author's risk (weakest: it makes the declaration cross-check advisory for exactly the values that carry the most structure). |
Not a release blocker — the current behaviour is fail-safe either way (an absent key errors → REVIEW). Gates nothing in D-156: it touches only the second spelling of cross-manifest sub-shape B2, which is struck on its first spelling (a purpose-built provider) regardless, and the graph-relationship shape needs only a flat cardinality: set fact. Relevant to anything that later publishes the provider-author guide (DEM-S02). |
Found writing the tier-1 ceiling record (E11-S01); recorded in D-156. Evidence: schemas/provider/v1alpha1/response.schema.json (declaration.type/cardinality, unconstrained value), internal/provider/resolve.go ResolveFactsChecked, internal/provider/builtin/repo_file.go (answerRepoFile, readMapping), internal/core/aggregate/evaluate.go factsToCEL/toCEL, internal/lint/facts_ref.go (checkFactsShape, selectChainFields) |