Repository navigation
docs(adr): a guard must be able to fail, and must know what it covers - #460
Conversation
ADR-0044 records what the SDK maturity program learned about guard tests: more than twenty review findings in a day were one defect -- a rule that covers the paths someone enumerated -- and the three fixes that broke the cycle share a property worth naming. Key on where a decision is made, ask the thing itself, remove the capability; and a guard must know its own reach, because a walk that visits nothing reports nothing, exactly like a clean tree. That last rule is enforced here rather than only stated: the shared walkers in internal/detectors/guards_test.go and the flat read in TestExportNeverReadsResolvedURL fail when they scanned no Go files. CLAUDE.md and AGENTS.md point at the ADR from their guard bullet. Closes #453. Co-Authored-By: Claude Fable 5.1 <[email protected]>
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
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 |
Bomly Diff SummaryCompared Overview
Dependency Changes✅ No dependency changes. Vulnerabilities✅ No vulnerability changes. License Changes✅ No license changes. Project Posture✅ No project posture changes ( Policy Findings✅ No policy differences were identified. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6002c9316c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… export guard walks recursively Codex review on #460: the shared walker's count is over all of internal/, so a scoped rule could inspect nothing while the walker stayed busy; the two direct detector walkers had no count at all; and the export guard read internal/sbom flat, so a codec moved into a subpackage would leave the rule. Each guard now counts what it actually inspected, and the export guard walks its whole scope. Co-Authored-By: Claude Fable 5.1 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 517639c4d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s its reach Codex review on #460: the guard read the package directory flat, so a parser added in a subpackage could call sdk.NewModuleNode unseen while the top-level files kept it green. It now walks the tree and fails when it scanned nothing, the rule ADR-0044 states. Co-Authored-By: Claude Fable 5.1 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a92c5fff47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review on #460: a root whose production files moved out still exists and still walks, and the other roots kept the traversal looking busy. Each root now fails when it yields no production Go file. Co-Authored-By: Claude Fable 5.1 <[email protected]>
1 participant
Closes #453.
ADR-0044 (
dev-docs/adr/0044-a-guard-must-be-able-to-fail-and-know-its-reach.md) records what the SDK maturity program learned about guard tests. Context: the five-round progression on the purl-type guard and the parallel one on the SDK's wire-schema guard, each fix opening the next hole. Decision, as seven rules: key on where a decision is made, not where it is used; ask the thing itself; prefer removing the capability to inspecting its arguments; a guard must know its own reach; exemption lists invert and stale entries fail; mutation is the acceptance criterion, with its traps named; two correct rules pulling against each other means the frame is wrong. Every rule cites the real test that embodies it.Enforced, not only stated:
walkInternalGo,filesNamingModuleandTestExportNeverReadsResolvedURLininternal/detectors/guards_test.gonow fail when they visited zero Go files, in the style ofTestGoldensCarryNoHostArchitecture. Verified by pointing the walker at an empty temporary directory: it fails with "the guard scanned nothing" (not committed).CLAUDE.md/AGENTS.md: the "Add a guard when the rule can be bypassed by writing it out by hand" bullet gains one sentence pointing at the ADR; the two files stay byte-identical past their headers.make verifypasses.🤖 Generated with Claude Code