Repository navigation
ci(feedback): record three gate gaps found adopting shield - #23
Merged
Merged
Conversation
Adoption run against nginx-http-shield-module, already at 3/3 markers with anchor 872fc33. The forward path found no candidate, so every finding is of the shape "the target has a gate this repo does not". All three are ports rather than fixes to existing files, which is why they are described here instead of changed in this commit: adding checkers to ci/linter/ and ci/tools/ changes the gate set of every module that adopts this skeleton next, and two of the three need a scope decision first. The runner one is the sharp finding. check_runners validates a runs-on selector against an allowlist of label sets, but the reason the allowlist exists is trigger-shaped -- a pull_request job executes scripts from the PR head. The check never reads the trigger, so an approved label set used without the fork ternary passes.
WalkthroughThe PR adds an adoption feedback document. It records three unported CI safeguards, their affected paths, observed costs, implementation options, and the decision not to add fuzz-dictionary tooling. ChangesCI safeguard adoption
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d2ca2f97-a38f-43a1-b6fe-98669b3b6c2e
📒 Files selected for processing (1)
ci/feedback/nginx-http-shield-module-2026-08-05.md
Both from the CodeRabbit review on #23. "holds by construction" implied an enforced invariant in the same sentence that said nothing enforces it. The point of the finding is that the property is unguarded, so the wording was arguing against itself. "costs nothing visible" collided with the 23 -> 35 signature-reach figure quoted 15 lines further down. What an incomplete dictionary actually does is pass the crash-only gate, which is narrower and true.
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.
Three gaps, one file, no code changed. They are ports rather than fixes to existing files, which is why the PR describes them instead of making them: dropping new checkers into
ci/linter/andci/tools/changes the gate set of every module that adopts this skeleton next, and two of the three need a scope decision first.The runner finding is the one worth reading.
check_runnersvalidates aruns-onselector against an allowlist of label sets (workflow_policy.py:87), but the stated reason that allowlist exists is trigger-shaped: apull_request-triggered job checks out and executes scripts from the PR head (:78-80). The check never reads the trigger. It asks whether the selector has an approved shape, so an approved label set used without the fork ternary passes the membership test at:243. The target closes this with a 64-line trigger-based script that is label-set independent.The other two: nothing asserts that a
workflow_callmember carries nopush:(the current workflow set happens not to contain one, and nothing stops the next copied workflow from bringing it back), andci/fuzz/fuzz.dictis hand-maintained with no drift gate. The third proposes no change to this repo's dictionary, only a sentence inPROMPT.mdstep 27, since the skeleton's patterns are illustrative and a generator here would be scaffolding for a table that does not exist.Testing
Docs only. Every claim cites a
file:linein this repo, verified against the current tree:ci/linter/workflow_policy.py:460—COMMANDSis{runners, ports, docs}; no cadence check existsci/linter/workflow_policy.py:243—runner in TRUST_SPLITS, membership test with no trigger readls ci/tools/check-workflow-runners.sh ci/linter/lint-ci-cadence.sh ci/tools/gen-fuzz-dict.py— all absent here, all present in the targetNo workflow, script or gate is touched, so the gate set is unchanged.