Repository navigation
Address admission and security CodeRabbit findings - #1055
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Scope contract and references internal/openapiimport/model.go, expr/security_scopes.go, expr/tla/security_requirements/README.md, .agents/skills/loom-framework/references/formal-models.md |
The OpenAPI model comment describes OAuth scopes and application roles in OpenAPI 3.1 and 3.2, and empty non-OAuth scope lists in OpenAPI 3.0. dupSchemeRequirements moves after ScopeNames without an implementation change. The README formatting and formal-model index are updated. |
Concurrency admission tests and examples
| Layer / File(s) | Summary |
|---|---|
Admission test checks admission/concurrency_test.go, codegen/generator/testdata/concurrency_admission_test.go.txt |
Selected test assertions change from fatal to non-fatal. A helper checks overloaded service errors. The generated test closes its finish channel before constructing the service. |
Admission example instructions docs/examples/concurrency/main.go.txt, docs/admission.md |
The example logs when work starts. The instructions require confirming that both calls acquired permits before sending a third request. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Possibly related PRs
- CaliLuke/loom#1050: This pull request updates the OAuth flow model index and the OpenAPI scope and role contract referenced by that PR.
- CaliLuke/loom#1046: This pull request changes admission-test assertions and fixture behavior, and updates the admission example instructions.
Merge Risk: 
5f881
-
No actionable correctness, security, availability, or documentation risk is established by this PR.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly identifies the two main areas of change: admission and security findings. It is concise and related to the pull request. Description check The description accurately explains the concurrency test fixes, overload demonstration changes, security documentation updates, and validation performed. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (5 skipped: 5 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the permits in a row.
Two calls begin; their start-time logs now show.
A third waits until the first two hold.
Test checks keep running when a fault is told.
Scope notes mark the roles and paths in view.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
CodeRabbit identified unsafe fatal assertions in concurrency test workers, a nil-channel hang on unexpected service execution, and a race in the documented overload demonstration. Use non-fatal worker assertions, let rejected-call regressions reach their assertions, and wait for explicit service-start messages before the third demo request.
A separate commit addresses the security review: correct the normalized scope/role field contract, place the OAuth model in the model table, format model identifiers, and move the private copy helper below public methods without changing its body.
Addresses all seven actionable findings on #1046 and #1050, including the outside-diff field comment. The other release ticket PRs and #1054 had no actionable CodeRabbit findings at audit time.
Validation: concurrency race tests; generated HTTP/gRPC example compilation and race tests; focused expression/importer security tests; canonical lint; full root test suite and coverage through the normal pre-push hook. Both exact commit diffs received independent review, with all findings fixed and re-reviewed. Normal commit and push hooks remain enabled.