Uh oh!
There was an error while loading. Please reload this page.
Require a policy decision to grant, and give callers a gate that fails closed - #31
Merged
Merged
Conversation
…s closed Two gaps, both of the same shape: the rule was stated where nothing enforced it. enable() accepted policyDecisionRef = null. "enabled" is the only state that authorises use — every other state describes an observation — so recording it without naming the decision behind it produces a ledger entry asserting an adjudication that may never have happened, and one that cannot afterwards be told apart from a properly adjudicated grant. It is now required. deny() deliberately stays permissive. A refusal that cannot cite its policy is still a refusal and still fails closed; a grant that cannot cite its policy is an unattributed authorisation. The two mistakes do not cost the same, so they are not gated the same, and the asymmetry has its own test so a later reader does not "fix" it into symmetry. isEnabled() carries the instruction "feature use must be gated on this before proceeding" in a docstring, and returns a boolean. A caller who never asks proceeds exactly as if permission had been granted — the requirement fails OPEN, enforced by whoever happened to read the comment. assertEnabled() throws instead, naming the state actually recorded and the policy decision behind it, so a refusal is diagnosable rather than opaque. Writing the negative controls found a bug in the change itself: getState() reports an unknown capability as null, not undefined, so the "not in the ledger" branch was unreachable and an undeclared capability would have produced a confusing 'is "null", not "enabled"' message. The test that exercises every non-enabled state is the one that matters — a capability part-way through its lifecycle must refuse, not pass through. Seven existing tests called enable() with no policy reference. They were exercising reconcile, conflict and timestamp behaviour rather than the permissiveness of enable, so each now names the decision that authorised it.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes the last two items on the declared-but-unenforced register. Both are the same shape: the rule was stated where nothing enforced it.
enable()acceptedpolicyDecisionRef = nullenabledis the only state that authorises use — every other state describes an observation. Recording it without naming the decision behind it produces a ledger entry asserting an adjudication that may never have happened, and one that cannot afterwards be told apart from a properly adjudicated grant.Now required.
deny()deliberately stays permissive. A refusal that cannot cite its policy is still a refusal and still fails closed; a grant that cannot cite its policy is an unattributed authorisation. The two mistakes do not cost the same, so they are not gated the same. That asymmetry has its own test, so a later reader doesn't "fix" it into symmetry.isEnabled()fails openA requirement stated in a docstring, returning a boolean. A caller who never asks proceeds exactly as if permission had been granted — enforced by whoever happened to read the comment. Its only callers in the entire repo were its own tests.
assertEnabled()throws instead, naming the state actually recorded and the policy decision behind it, so a refusal is diagnosable rather than opaque.isEnabledis kept and its docstring now says plainly that it fails open.The negative controls found a bug in the change itself
getState()reports an unknown capability asnull, notundefined. My first version checked=== undefined, so the "not in the ledger" branch was unreachable — an undeclared capability would have produced a confusingis "null", not "enabled". The test caught it before the commit.The test that matters most iterates every non-enabled state (
declared,requested,negotiating,available,degraded,unsupported_by_runtime,missing_plugin,failed) and asserts each refuses. A capability part-way through its lifecycle must not pass through.Note on the existing tests
Seven called
enable()with no policy reference. They were exercising reconcile, conflict and timestamp behaviour — not the permissiveness ofenable— so each now names the decision that authorised it. That they all needed changing is the measure of how freely an unattributed grant could be recorded.38 tests, 0 failures (30 before, 8 added).
🤖 Generated with Claude Code