refactor(provisioning): readability + maintainability sweep#71
Merged
Conversation
…rt assertion Extract CEL environment construction and ref.Val helpers into cel.go; condition.go keeps the condition lifecycle (compile/cache/match/validate). Rename ResolverOptions.RuleSource to RuleLister to match the authkit.ProvisioningRuleLister port name. Add the missing compile-time PrincipalResolver assertion on *Resolver. Expand the package doc to spell out the wrap-and-conditional-provision contract. Cascade: testkit/internal/authflow/runtime.go updates the struct-literal field name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Godoc every private function and module-scope global, replacing the bare nolint justifications on conditionEnvironment and conditionProgramCache with real docs. Annotate the four CEL fail-closed gates (eval error, non-bool runtime output, AST output-type check, CostLimit/InterruptCheckFrequency bounding) and the two provisioning gates in ResolveIdentity (ErrUnresolvedIdentity-only triggers provisioning, factory denial preserves the original unresolved error). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move test fixtures and hand-rolled fakes into helpers_test.go; move TestMatchRules and TestValidateConditionRejectsInvalidExpressions into rules_test.go; resolver_test.go keeps the constructor and ResolveIdentity cases. Mechanical lift — no behaviour change. Drop TestResolverSatisfiesPrincipalResolver: the production-side var _ authkit.PrincipalResolver = (*Resolver)(nil) assertion added earlier in this PR subsumes it, matching the PR #62/#68 drop precedent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace fakeResolver, fakeProvisioner, and fakeRuleSource with the
authkitmocks.{PrincipalResolver,IdentityProvisioner,ProvisioningRuleLister}
constructors that mockery already generates. Each test configures the
specific expectation it needs and passes unconfigured mocks for ports its
short-circuit path never reaches so stray calls panic rather than silently pass.
No .mockery.yaml change; all three entries existed before this PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
provisioning/to the same 10-criteria bar as PRs refactor(access): readability + maintainability sweep + introduce mockery #61–refactor(proof/passkey): readability + maintainability sweep #69. Ninth of ten package sets — onlystore/{memory,postgres}/andtestkit/remain.condition.go(277 LOC) intocondition.go(condition lifecycle) +cel.go(env + ref.Val helpers). Adds godocs to every private helper across both files plus inline comments on the four CEL fail-closed gates and the twoResolveIdentityprovisioning gates.ResolverOptions.RuleSource→RuleListerto match theauthkit.ProvisioningRuleListerport name; cascades to one production consumer intestkit/internal/authflow/runtime.go.var _ authkit.PrincipalResolver = (*Resolver)(nil)assertion and dropsTestResolverSatisfiesPrincipalResolver(subsumed, per PR refactor(authz): readability + maintainability sweep #62/refactor(proof/oidc): readability + maintainability sweep #68 precedent).resolver_test.go(440 LOC) intoresolver_test.go+rules_test.go+helpers_test.go(mirroring onboarding/ from PR refactor(onboarding): readability + maintainability sweep #66); migrates the three hand-rolled fakes (fakeResolver,fakeProvisioner,fakeRuleSource) to the already-generatedauthkitmocks.*constructors..mockery.yamlunchanged.RuleSource→RuleListerrename.Test plan
moon run root:check --summary minimalgreen per commit (format, lint, build, unit, Testcontainers integration)testkit/internal/authflowintegration suite covers the renamedRuleListerfield via the full auth flowTestCompileConditionCachesNormalizedProgramstill passes after thecondition.gosplit (cache type stayed incondition.go)🤖 Generated with Claude Code