🔒 fix(cmd): discriminate absent provider declaration from forge failure (REL-03) - #78
Merged
Conversation
…re (REL-03)
resolveRunFacts treated ANY FileAtRef error on providers/<name>.json as
"declaration absent" and continued, so a retry-exhausted 5xx, a throttle or a
token scoped away from the governance repo was indistinguishable from "this
provider declares nothing". The fail-safe DIRECTION held — no fact binds, CEL
sees an absent attribute, the run degrades to REVIEW — but the PATH was
invisible: the operator got a missing-attribute predicate.error naming nothing,
and a has()-tolerant policy silently took its fallback branch. A wrong decision
reached without a diagnostic.
continue now happens only for errors.Is(err, forge.ErrNotFound), the neutral
port sentinel every adapter wraps with %w. Every other error ends the run with
an error naming the provider, the declaration path and the ref.
No new discriminator idiom: this is D-130's ruling applied to its sibling call
site. loadResourceOwnerRegistry (same file) fixed exactly this conflation for
the who-may-approve registry; fileAtRefOrAbsent (run.go) is the same shape.
Hard-failing is also the consistent choice locally — an UNREADABLE declaration
now ends the run exactly as a MALFORMED one already did six lines down
(GUIDELINES Safety 2).
REQ-AUD2-S02-01 absence still skips (through the real gitlab 404 wrap chain)
REQ-AUD2-S02-02 5xx returns an error naming provider, path and ref
REQ-AUD2-S02-03 401/403 returns an error; a scope misconfiguration is never absence
REQ-AUD2-S02-04 non-vacuity: reverting the guard to a bare continue reddens -02
and -03 while -01 stays green
REQ-AUD2-S02-05 no shipped decision outcome changes; task check (which runs
dogfood-examples over all three packs) exits 0
REQ-AUD2-S02-02 asks the error to name the provider, the declaration path AND the ref. The first version checked those as three separate strings.Contains calls — but brokenForge (the fake's adapter-shaped error) already renders the path and the ref inside the wrapped cause, so two of the three assertions could not fail: a bare `provider %q: %w` wrap would have passed them. The test was measuring the fake, not the code. Asserted now as ONE contiguous substring that only the outer wrap can produce, plus a separate check that the forge's own cause survives the %w. Non-vacuity re-proved: mutating the implementation to `provider %q: %w` reddens TestProviderDeclarationForgeErrorAbortsResolveRunFacts and nothing else. The first version of the test stayed GREEN under that same mutation. Also corrects the code comment: the malformed-declaration hard-fail this change is made consistent with is in the very next statement (LoadProviderConfig), not "six lines down" as the previous commit body and comment both said.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AUD2-S02 — REL-03
ErrNotFounddiscriminationresolveRunFactsincmd/assent/provider_host.goused tocontinueon any error fromthe forge file read, so a 503, a throttle, or a token scoped away from the governance repo was
indistinguishable from "this provider declares no host". The fail-safe direction held (no fact
binds → CEL sees an absent attribute → REVIEW), but the path was invisible: the operator got a
missing-attribute predicate error naming nothing, and a
has()-tolerant policy silently took itsfallback branch.
Now only
errors.Is(err, forge.ErrNotFound)— the neutral port sentinel every adapter wraps with%w— is treated as absence; every other error ends the run with a wrapped, named diagnostic,consistent with how a malformed declaration already fails in the very next statement.
Changes
cmd/assent/provider_host.go— discriminate absence from failure (REL-03 / D-130).cmd/assent/provider_host_test.go— both polarities, plus the error-wrap assertion.CHANGELOG.md— Integrator-owned regeneration (task changelog-write).Gate
CI=true task checkexits 0 on this rebased head, includingchangelog-verifyandrelease-changelog-gate-test.Independent review: APPROVE, no P0/P1, no merge conditions.
Branch note: opened from
lane/aud2-s02-integraterather thanlane/aud2-s02because the rebaseonto
origin/mainplus the CHANGELOG amend rewrote the published lane SHAs, and force-push is notauthorized in this repo.